Skip to content
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

Change some 'RC' keywords to more appropriate 'manual_control' if also valid for joystick #23640

Merged

Conversation

sfuhrer
Copy link
Contributor

@sfuhrer sfuhrer commented Sep 3, 2024

Most of all the arming/disarming reason should be "Stick gesture" and not "RC".

@sfuhrer sfuhrer requested a review from MaEtUgR September 3, 2024 16:34
Copy link
Member

@MaEtUgR MaEtUgR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for bringing this up. The wording is actually wrong.
The only thing I'd do differently is take a term e.g. "stick gesture" and then use that term everywhere and not have the one enum "manual_control" and the UI string "stick" and another enum "rc" for the same thing.

I can add a suggestion.

src/lib/events/enums.json Outdated Show resolved Hide resolved
src/modules/commander/Commander.cpp Outdated Show resolved Hide resolved
src/modules/commander/Commander.cpp Outdated Show resolved Hide resolved
src/modules/logger/params.c Outdated Show resolved Hide resolved
MaEtUgR
MaEtUgR previously approved these changes Sep 5, 2024
Copy link
Member

@MaEtUgR MaEtUgR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sfuhrer I thought "joystick gesture" is even more clear. What do you think?

@sfuhrer
Copy link
Contributor Author

sfuhrer commented Sep 5, 2024

@sfuhrer I thought "joystick gesture" is even more clear. What do you think?

Not necessarily. I associate the word "joystick" to a gamepad generally, so a source for manual control inputs not being an RC. That's also how the groundstation tab is called:
image
So I would prefer to go back to "stick".

@MaEtUgR MaEtUgR force-pushed the pr-commander-change-arming-string-to-stick-arming-main branch from 2dc567e to e7ec5f5 Compare September 5, 2024 15:40
Copy link
Contributor Author

@sfuhrer sfuhrer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the add-ons @MaEtUgR !

Copy link
Member

@MaEtUgR MaEtUgR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

associate the word "joystick" to a gamepad

You're right, that would be confusing again.

Just blew my mind how inaccurate humans including me usually are with wording 😄
How's a joystick a gamepad but not the stick of a remote? Does the prefix joy change it?
Because there were gameport "joystick"s for flight simulators in the 90s?
I digress 😇

@sfuhrer sfuhrer merged commit f98eb06 into main Sep 5, 2024
55 checks passed
@sfuhrer sfuhrer deleted the pr-commander-change-arming-string-to-stick-arming-main branch September 5, 2024 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants