registers: support writing axis acceleration value #337
Conversation
|
All values in bytes. Δ Delta to base
|
|
🤔 does it really work? I supposed we didn't have those runtime accelerations used in the code. |
3d-gussner
force-pushed
the
acceleration-registers
branch
from
b97c64c to
0f4e7d6
Compare
Automated Test Code Coverage ReportView details...
TOTAL: 2689 lines of code, 1000 lines executed, 37% covered. |
|
This PR makes #334 obsolete, right? |
|
while |
The read operation now returns the actual used value instead of the maximum allowed acceleration value. Change in memory: Flash: + 78 bytes SRAM: 0 bytes
gudnimg
force-pushed
the
acceleration-registers
branch
from
0f4e7d6 to
75a5ba3
Compare
|
Rebased to sync with main. No functional changes made. |
@3d-gussner If you find time, can you double check you were using the correct firmware? I don't see how the write can be rejected. It should work now when we have a 'write' function defined. I'll see if I can test this on my MK4S today or tomorrow. For reference, this is the function which determines if its accepted or rejected. False means Rejected.
|
When reading or setting the value, the driver is expecting steps_t which is a axis scaled value. This increases the memory footprint quite a bit. But now if you set 800mm/s2, you should get a similar value back. Flash: +132 bytes SRAM: 0 bytes
When converting 800mm/s2, it would be truncated to 795mm/s2 for the pulley. This is due to cutting out significant decimal digits. Instead let's multiply in floating point, this needs quite a bit of resources. So to optimise against this, multiply with the recoprical. Then the cost is not more than 20 bytes. Testing: M707 A0x0e; Read Pulley Acceleration (default at boot up is 800mm/s2) M708 A0x0e X790 ; Set Pulley Acceleration to 790mm/s2 M707 A0x0e; Read Pulley Acceleration (should be 790mm/s2) The results before this commit: M707 A0x0e -> returns 805 M708 A0x0e X790 ; Set Pulley Acceleration to 790mm/s2 M707 A0x0e; returns 795 After this commit: M707 A0x0e -> returns 799 M708 A0x0e X790 ; Set Pulley Acceleration to 790mm/s2 M707 A0x0e; returns 789 NOTE: axisUnitToTruncatedUnit is used in Idler homing, selector homing, and pulley positioning. I am not sure yet how this improvement will affect those areas.
|
The unit tests need updating after the last commit, they encounter timeout since the tests don’t expect to see homing fail. For the acceleration registers this improved accuracy is only cosmetic. But it does impact idler/selector homing. I will continue to work on the unit tests to see if I can improve things. I don’t see any homing issues on real hardware. Writing and reading the acceleration registers work well on my end. |
|
@gudnimg After adding few more debug lines I can confirm that changing the accel register works. Thanks for the commit comment that writing Lot of the tests are still failing. 🚧 |
|
I can move aa20e01 into a separate PR. It is not strictly necessary for supporting writing acceleration register. Its only the read value which will be off by a few mm/s2. Not sure how long it will take to resolve the unit tests. Its not trivial. |
|
I plotted the trunction error produced by Pulley:
Selector:
Idler:
|
The idler is only put on hold, and resumed after a user event is seen. We must ensure the idler is engaged before feeding to FINDA.
We never home Idler and Selector at the same time. I think we did early perhaps, but now the selector always waits for the idler homing to be valid.
Add handling for HomingFailed which can cause the tests to timeout. Add checks for HomeForward and HomeBack
Before, the axis limits had an error of 25° so 225° would actually be measured as 250°. Now after fixing axisUnitToTruncatedUnit to return a more accurate value, the new values are lower. Adjust the distances by 20° to offset previous error.
|
Now the unit tests pass :) |
| @@ -64,7 +64,7 @@ void Idler::FinishMove() { | |||
| } | |||
|
|
|||
| bool Idler::StallGuardAllowed(bool forward) const { | |||
| const uint8_t checkDistance = forward ? 220 : 200; | |||
| const uint8_t checkDistance = forward ? 200 : 180; | |||
There was a problem hiding this comment.
I wonder if we can tighten these values closer to 225°
|
So, it's working nicely on the real HW ... actually it's funny to watch very slow accelerations. |
axisUnitToTruncatedUnit) by using floating point multiplication. This only costs 14 bytes of Flash.Fixes #333
Change in memory:
Flash: +224 bytes
SRAM: 0 bytes