Skip to content
This repository was archived by the owner on Nov 3, 2024. It is now read-only.

WIP : Flash rework - #120

Open
jlmxyz wants to merge 11 commits into
CAP1Sup:devfrom
jlmxyz:FlashRework
Open

jlmxyz wants to merge 11 commits into
CAP1Sup:devfrom
jlmxyz:FlashRework

Conversation

@jlmxyz

@jlmxyz jlmxyz commented Apr 3, 2022

Copy link
Copy Markdown

this is a branch created for :
use a real flash structure so that flash content can be
1- versioned (different from firmware version, as the flash structure can evolve but firmware features didn't(flash reorg)),
2- dumped (in JSON format, so that a pc can get the parameters before firmware upgrade and convert them to re-set),
3- have builtin CRC to prevent corruption or partial flashing
4- avoid duplication of flashed parameters in motor (use dedicated reference to access the flashed parameters) and also avoid some "cached" precomputed values that seems not to be updated when set on flash or oled menu
5- allow de-activation of dip switches since they will override microstepping set on flash....
6- add some testings scripts

/!\ change of G6 direct stepping command, as is was not moving of a step (a step is hardware stepping (eg 1.8 or 0.9 angle) * micro stepping)

this is a WIP, I need some help because I have some issue when testing, when I set the microstepping to 1:1 and ask for a single step :
1- sometimes the motor move, sometimes not (my motor is 1.8 angle)
2- seems that the direction is "random" (about 90% of time direction has not impact, it move clockwise, but sometimes it follow the direction and move back....)

I still have also another problem when running the move5cm.sh test, the direction is correct, but each move is preceded by a "fast rewind in opposite direction of the same distance" eg if I run the script that ask for +5cm then -5cm, the current behavior is -5cm fast foward, +5cm at requested rate, +5cm fast forward, -5cm at requested rate....

@jlmxyz jlmxyz mentioned this pull request Apr 3, 2022
@CAP1Sup

CAP1Sup commented Jun 6, 2022

Copy link
Copy Markdown
Owner

Is this still a WIP? It seems like from your comments it isn't finished. I do have some minor adjustments that I'd like to look at, but overall I think that this PR is definitely a step in the right direction. The whole flash organization is much better than my original code. I never really got into struct or union, so it's nice to see them used to really clean up the code.

@jlmxyz

jlmxyz commented Jun 24, 2022

Copy link
Copy Markdown
Author

from my point of view, it's finished, there might be some discussion about the versioning of the flash that is different from the versioning of the app, and might be some improvement about versioning version conversion (using dumping and script to convert from one to another) but there is also the "all checks have failed" about the merge request that I didn't look into

and I have issue moving the motor, but not sure that it's related to my changes, since I use a V57B to perform my tests (I should shortly be able to test on V42B with a 0.9 stepper) at this time, when I ask for a 5mm move, it move backward fast backward 5mm then 5mm forward at requested speed....

there is also changes that you should look into, since I modified the move command to use "real stepping" and not "full angle stepping" (real stepping = a micro step, full angle = 1.8 or 0.9 according to settings so several micro steps)

@jlmxyz

jlmxyz commented Jun 24, 2022

Copy link
Copy Markdown
Author

please note some important points in flash.h :
#pragma pack(push, 1) <= this will prevent the compiler to "pad" some data for speed optimisation, which can't be controled and might cause flash to be detected as corrupted

// Note that the flash CANNOT store more than 32 parameters <= this comment is wrong, a page size is FLASH_PAGE_SIZE (1ko or 2ko according devices) so about 256 32bits words if no optimisation

the actual sanity check is done by BUILD_SANITY_CHECK_flashStorageSize

@CAP1Sup

CAP1Sup commented Jul 1, 2022

Copy link
Copy Markdown
Owner

I apologize for the delay in reviewing your PR, I'm hoping to have some time this weekend to make some comments and do some testing. Like I said, I just returned from Spain and I'm in the process of catching up

@CAP1Sup CAP1Sup added the improvement Improvement on current performance label Jul 1, 2022
@jlmxyz

jlmxyz commented Aug 18, 2022

Copy link
Copy Markdown
Author

I found something interesting : to be sure that the commands were handled and sync the test scripts, I modified the return string to add the string "CMD END" where CMD is the Gcode sent
I noticed that if I include the start and stop markers in the reply I create a endless loop, using debugger I noticed that the rx buffer of serial line contains the text sent in reply (the rx and tx buffer are alike with offsets)....

so now I use the start and stop markers in commands sent to the S52B and reply without the markers and can go further on testing, i've 100% reproductibility of the bug where the motor first move slowly in reverse direction then perform the move....

@jlmxyz

jlmxyz commented Aug 18, 2022

Copy link
Copy Markdown
Author

I continued my investigation about the move bug....
I put some breakpoints in void StepperMotor::step(STEP_DIR dir, int32_t stepChange) and in timer callback
void stepScheduleHandler()

1- the callback is called as long as remainingScheduledSteps >= 0 and motor perform step each time
2- once remainingScheduledSteps == 0 the callback is no more called but StepperMotor::step is called

looking at stepScheduleTimer object all the channels are in TIMER_DISABLED but TIM4_IRQHandler is still called... but somehow it is called the same number of step than the count requested... and I don't know the function that is called to disable the timer at the end.... if you have a hint, it will be greatly appreciated

@jlmxyz

jlmxyz commented Aug 18, 2022

Copy link
Copy Markdown
Author

it's Timer1 that move back the motor.... Timer4 is the one that is driving the motor movements on Gcode commands but Timer1 seems to be still active...

@jlmxyz

jlmxyz commented Aug 18, 2022

Copy link
Copy Markdown
Author

ok, I found, now the motor don't move back after a G6 command, I still have a test to fix and the automerge check also, and try tests on 0.9 motor before it can be reviewed and merged

@jlmxyz
jlmxyz marked this pull request as ready for review August 19, 2022 11:33
@jlmxyz

jlmxyz commented Aug 19, 2022

Copy link
Copy Markdown
Author

now everything is working appart github checks, don't know how to fix them

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

improvement Improvement on current performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants