thingswebuilt / osod24_firmware

0 stars 0 forks source link

Enable zeroing odometry (heading and X/Y) and assign them to transmitter channels #54

Closed markmellors closed 4 months ago

markmellors commented 4 months ago

The methods work, but I've had to add a sleep to stop them double-triggering before the values have propogated through the system, and effectively reverting what's just been set. They work as they are and the time delay isn't currently a problem since we're setting them manually whilst stationary.
I've added some notes about this, I wonder if it'd be better to add a timed lock to the functions to prevent the double triggering (so that other functions aren't blocked if we use the methods in autonomous mode whilst driving), or if there's a better way to zero/apply offsets that doesn't suffer from the double-triggering problem? Hopefully the potential double triggering failure mode is fairly clear. For setOrigin, I think it's because it uses Navigator's current_state property which takes some time to be updated, so if you call it twice in row before the values have propogated, you first set the origin to zero, then the second time you offset it again, effectively to minus the previous values.

markmellors commented 4 months ago

also addresses #35 and #31 and the final task of #3 ?

markmellors commented 4 months ago

I've made the changes we discussed (they seem to work well!), so ready for another / more detailed review

robberwick commented 4 months ago

Has some conflicts to resolve, and then it's mergeable

robberwick commented 4 months ago

to save me re-reviewing this again, i'm happy if it passed a manual test by trying out the build on the robot. if you want to do that and then hit the merge button when you're satisfied that it's working, that works for me 👍

markmellors commented 4 months ago

tested, works