Refactor AngularVelocity/Acceleration classes#3541
Refactor AngularVelocity/Acceleration classes#3541nycrat wants to merge 13 commits intoUBC-Thunderbots:masterfrom
Conversation
4fe3dc7 to
20c46a6
Compare
|
These comments also apply to the angular acceleration files. |
|
drive-by review, another good and valid option that I'd argue is cleaner is to declare separate AngularVocity and AngularAcceleration classes that internally hold an Angle under the hood: The benefits are that you get type-safety without duplicating a lot of code and you only need to specify the functions that are relevant for the class. Inheritance doesn't really make sense here because conceptually AngularAcceleration is kinda different from an Angle. |
|
Yea maybe avoiding inheritance is the best solution, I'll see what I can do |
8aafa04 to
411120c
Compare
411120c to
a36cb87
Compare
f90fb78 to
b9601d0
Compare
13111fa to
75b7521
Compare
|
Ok this new solution uses C++ 20 concepts and abbreviated function templates to make the generic angle class much cleaner and it is easier to change the behaviours of methods for specific classes. I fixed all the type issues related to Angle and AngularVelocity so the software and simulated gameplay tests all pass now. However, I think that the cross compiler for the jetson/raspberry pi has an outdated gcc version that doesn't completely support these c++ 20 features, not exactly sure how to fix that... apparently it comes from https://github.com/UBC-Thunderbots/Software-External-Dependencies? |
b29edec to
af512db
Compare
Description
Instead of the AngularVelocity and AngularAcceleration literally being an alias of Angle, this PR refactors them into child classes of Angle which has a few advantages:
Line 7 of this code shows exactly the reason why original aliases were an issue.
Testing Done
Ran thunderscope and the tests, everything seems to be fine.
Resolved Issues
resolves #3093
Length Justification and Key Files to Review
Lots of the changes were to correct usage of Angle -> AngularVelocity in lots of files in the codebase.
Review Checklist
It is the reviewers responsibility to also make sure every item here has been covered
.hfile) should have a javadoc style comment at the start of them. For examples, see the functions defined inthunderbots/software/geom. Similarly, all classes should have an associated Javadoc comment explaining the purpose of the class.TODO(or similar) statements should either be completed or associated with a github issueNOTE: I'm not sure if this is the best approach, it is possible to privately inherit from Angle and then avoid the delete statements so that may be cleaner instead. But it would be necessary to redefine inherited functions such as toRadians() that just calls the parent function. It is already necessary to redefine parent functions that return Angle, so for example, we need to redefine fromRadians(rad) since the original returns an Angle, and we need it to return AngularVelocity instead.
Also I'm not sure exactly which methods should be included or not used by the angular velocity/acceleration classes.