-
Notifications
You must be signed in to change notification settings - Fork 610
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[glass] Add math expression input for NetworkTables numerical values #6530
[glass] Add math expression input for NetworkTables numerical values #6530
Conversation
/format |
This is amazing work! It will take me a bit of time to fully review the expression parsing code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, looks pretty good! Just have a few relatively minor comments.
I think this is pretty much done now, do you have any other comments? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be worth it for Token
to store std::string_view
instead of const char*
and int
, but it doesn't matter much.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be really nice to have unit tests of the expression parser (e.g. does 1 + 2 ** 5 * 3
evaluate to 97), but glass doesn't have tests setup so it might not be worth it or be outside the scope of this PR.
Great work, though!
This is for easier adjustment of NetworkTables values when tuning values. For example, you can increase an angle by 5 degrees easily by adding
+5
to the value instead of doing the addition yourself. We implemented a similar feature in our custom dashboard program, which we found very convenient during the build season.