507 motor constants setter only called when full motor constants object is supplied instead of at attribute change#509
Conversation
…es update correctly
…R_CONSTANTS attribute
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
senthurayyappan
left a comment
senthurayyappan
left a comment
There was a problem hiding this comment.
Looks good. Overall, it looks like a non-breaking change, since all the changes are internal. Should we still test this PR on hardware before merging, just in case?
@VarunSatyadevShetty @ellliewilson Can I please get your help running this branch on your existing setup? Adding you both as reviewers to this PR!
|
Sounds good! I am happy to take a look at this PR! Thank You @senthurayyappan @tkevinbest |
|
@VarunSatyadevShetty I don't think this needs a full OSL hardware test. Could even just be on an actpack with a regular pi and make sure things work as expected? Or if someone has a test bench setup handy, we could validate it pretty easily if there are two different torque constants. In my opinion, since it passes the unit tests I think we can just merge it. |
3 participants
Updated handling of motor constants to properly call a callback if any value is modified. Updated all instances and tests accordingly.
Fixes #507 .
Also minor renaming of motor constants class to make it clearer what is a class and what is an instance.