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

Multi joystick functions profile #667

Conversation

rafaellehmkuhl
Copy link
Member

@rafaellehmkuhl rafaellehmkuhl commented Dec 27, 2023

image

I'm waiting for Tony or Kurt to confirm if the boat profile is correct (specially the Z axis mapping). Fixed with Tony's help.

Fix #636.

@rafaellehmkuhl rafaellehmkuhl marked this pull request as draft December 27, 2023 01:29
@rafaellehmkuhl rafaellehmkuhl force-pushed the multi-joystick-functions-profile branch from 0af36ea to dd4fb5f Compare December 27, 2023 21:52
@rafaellehmkuhl rafaellehmkuhl force-pushed the multi-joystick-functions-profile branch 2 times, most recently from 782d59f to 27e8039 Compare January 9, 2024 00:53
Comment on lines +42 to +50
<Button
v-for="functionMapping in controllerStore.protocolMappings"
:key="functionMapping.name"
class="m-2"
:class="{ 'bg-slate-700': controllerStore.protocolMapping.name === functionMapping.name }"
@click="controllerStore.loadProtocolMapping(functionMapping)"
>
{{ functionMapping.name }}
</Button>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the buttons would be more intuitive if they said something like "Restore default ROV controls" - general users don't need to know that we're calling them "functions mappings" internally, and the active "Restore" makes it clearer that the current mapping is going to be replaced by a different one, as opposed to the similarly styled modifier buttons directly below which just display different parts of the current controls mapping.

Tangentially related, having buttons for this that look the same as the buttons for the modified profiles (regular/shift) is unintuitive and somewhat unpleasant. That's probably out of scope for this PR though, since the preferred improvement is to change the modified profiles buttons into tabs (like in #617), and the new restore functions can be left as buttons.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree with almost everything. The restore I'm not sure about as this is actually not a "restore" button, but a switch between mappings. I agree thought that it is currently very non-intuitive. I want to change this whole interface once we get the needed functionalities on (I think we are only missing the 32 buttons support after this PR).

Copy link
Contributor

Choose a reason for hiding this comment

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

this is not actually a "restore" button, but a switch between mappings

That doesn't make sense to me - if it's changing the mappings to a default one (not determined by the user) then I would call that restoring (from their current customised one). If it's just two separate vehicle-type-specific profiles that the user has independent control over then we shouldn't have "default" in the button names.

Copy link
Member Author

Choose a reason for hiding this comment

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

They are determined by the user when he changes it. They have default in the name because they start as default, but I agree with you on this. Will remove the "default" from the name.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@ES-Alexander ES-Alexander added the docs-needed Change needs to be documented label Jan 9, 2024
@rafaellehmkuhl rafaellehmkuhl force-pushed the multi-joystick-functions-profile branch from 27e8039 to 8a5c6d2 Compare January 9, 2024 18:22
@rafaellehmkuhl rafaellehmkuhl marked this pull request as ready for review January 9, 2024 19:57
@rafaellehmkuhl rafaellehmkuhl force-pushed the multi-joystick-functions-profile branch from 8a5c6d2 to dd31d01 Compare January 9, 2024 20:01
@rafaellehmkuhl rafaellehmkuhl marked this pull request as draft January 10, 2024 19:58
@rafaellehmkuhl rafaellehmkuhl force-pushed the multi-joystick-functions-profile branch from dd31d01 to 6440af4 Compare January 11, 2024 18:09
@rafaellehmkuhl rafaellehmkuhl marked this pull request as ready for review January 11, 2024 18:10
@rafaellehmkuhl rafaellehmkuhl force-pushed the multi-joystick-functions-profile branch from 6440af4 to d4ebd40 Compare January 15, 2024 17:59
@rafaellehmkuhl rafaellehmkuhl force-pushed the multi-joystick-functions-profile branch from d4ebd40 to 50cd28d Compare January 16, 2024 19:54
@rafaellehmkuhl rafaellehmkuhl force-pushed the multi-joystick-functions-profile branch from 50cd28d to f84ecb4 Compare January 16, 2024 19:59
@patrickelectric patrickelectric merged commit 719f1b9 into bluerobotics:master Jan 22, 2024
7 checks passed
@rafaellehmkuhl rafaellehmkuhl deleted the multi-joystick-functions-profile branch January 22, 2024 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-needed Change needs to be documented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cockpit should allow selecting between different joystick functions mapping
3 participants