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

Disable Slide to Confirm on Arm and Disarm when Called from Joystick #830

Conversation

ericjohnson97
Copy link
Contributor

This PR disables slide to confirm on arm and disarm when called from the joystick.

These changes open the door to allowing the user to enable and disable slide to confirm from the widget customization

@patrickelectric
Copy link
Member

Maybe @JoaoMario109 is working on something that may be related

@ericjohnson97
Copy link
Contributor Author

ericjohnson97 commented Mar 15, 2024

Maybe @JoaoMario109 is working on something that may be related

I saw that this was added to slide to confirm, but I was unsure how to use it 😅.
https://github.com/bluerobotics/cockpit/blob/master/src/libs/slide-to-confirm.ts#L10

I think the ideal would be to have the configuration done per widget in the edit view, but I would like to hear what your plan currently is @JoaoMario109

@JoaoMario109
Copy link
Contributor

Maybe @JoaoMario109 is working on something that may be related

I saw that this was added to slide to confirm, but I was unsure how to use it 😅. https://github.com/bluerobotics/cockpit/blob/master/src/libs/slide-to-confirm.ts#L10

I think the ideal would be to have the configuration done per widget in the edit view, but I would like to hear what your plan currently is @JoaoMario109

Currently we've decided to allow users to globally enable or disable confirmation events in the mission configuration. This includes categories like arm, disarm, etc. Each "slide to confirm" call must confirm the relevant event category, with checks happening within the slide-to-confirm lib itself. We're also currently adding "hold to confirm" functionality, allowing cockpit actions to achieve the same outcome as sliding with the mouse by simply holding a button on the controller.
Regarding the proposed changes, I don't foresee any issues, but I'm concerned about user experience. For instance, having the 'arm' event category enabled by the user but not requiring this confirmation in some scenarios maybe could confuse the user. So maybe we need to consider specific use cases to determine whether it's better to maintain both, global categories "enable/disable" by users and allowing certain widgets to bypass it, or if we should change our current approach.

@ericjohnson97
Copy link
Contributor Author

Maybe @JoaoMario109 is working on something that may be related

I saw that this was added to slide to confirm, but I was unsure how to use it 😅. https://github.com/bluerobotics/cockpit/blob/master/src/libs/slide-to-confirm.ts#L10
I think the ideal would be to have the configuration done per widget in the edit view, but I would like to hear what your plan currently is @JoaoMario109

Sorry for going MIA for the last couple days.

Currently we've decided to allow users to globally enable or disable confirmation events in the mission configuration. This includes categories like arm, disarm, etc. Each "slide to confirm" call must confirm the relevant event category, with checks happening within the slide-to-confirm lib itself.

By "mission configuration" do you mean in the mission planning interface?

We're also currently adding "hold to confirm" functionality, allowing cockpit actions to achieve the same outcome as sliding with the mouse by simply holding a button on the controller.

This would be a cool feature and a good way to handle confirmation on a game pad. QGC does not have any type of confirmation on the commands sent from the gamepad which can be a bit dangerous.

Regarding the proposed changes, I don't foresee any issues, but I'm concerned about user experience. For instance, having the 'arm' event category enabled by the user but not requiring this confirmation in some scenarios maybe could confuse the user. So maybe we need to consider specific use cases to determine whether it's better to maintain both, global categories "enable/disable" by users and allowing certain widgets to bypass it, or if we should change our current approach.

Global categories may make sense, but also a configuration on each route would also make sense to me. maybe there could be a checkbox for hold to confirm in the joystick configuration interface as well as checkboxes for slide to confirm in each widget.

I personally don't have a strong preference here. The main issue I am trying to solve is trying to remove slide to confirm when the joystick commands something as it is not very ergonomic when I press arm on the joystick, I am also required to slide on the screen to confirm the action.

@JoaoMario109
Copy link
Contributor

Apologies for the late response, I haven't dedicated much time to this feature over the past week.

By "mission configuration" do you mean in the mission planning interface?

Regarding about "mission configuration," I'm referring specifically to the designated configuration tab within the configurations menu, accessible via the hamburger menu. This is similar to the "joystick configuration" page. Currently, it only includes the "hold to confirm" details.

Global categories may make sense, but also a configuration on each route would also make sense to me. maybe there could be a checkbox for hold to confirm in the joystick configuration interface as well as checkboxes for slide to confirm in each widget.

I currently have a draft PR intended for merging, but after discussion with @patrickelectric , we concluded it requires some refinement, particularly regarding the current implementation of the "hold to confirm" feature. This feature presently occupies a joystick button slot, an outcome we aim to avoid. The solution we are working on involves unifying the "hold to confirm" and "slide to confirm" functionalities under a single library, distinguishing them solely by their confirmation method, either via joystick or UI.

I think that right now it would be better to eliminate the necessity for users to comprehend and utilize a dedicated button solely for confirmation purpose. Probably would be more intuitive make something more similar with what you said, for example, within the joystick mapping configurations, users could enable a confirmation requirement for a particular action. This would mean, if a user maps the "X" button to perform the "arm" action and opts for confirmation upon mapping, pressing the "X" button would initiate both the "slide to confirm" on the UI or a prolonged press (e.g., 2 seconds) for confirmation. This approach addresses issues related to mapping specific buttons for confirmation and simplifies the process for users: if a button press requires confirmation, simply hold the button longer, and a slider will visually confirm the action.

Not sure if I covered all points here, but we gonna probably try to implement something like, this soon, maybe we can add these changes to allow the parameters for confirmation in arm and disarm, but probably soon we gonna need to change a bit because we probably will try to keep the decision within the "slide/hold to confirm" lib.

@rafaellehmkuhl
Copy link
Member

rafaellehmkuhl commented Apr 1, 2024

Just to add my 2 cents here, I agree it would be much better if the confirmation does not take a button slot. To do this, we can have a "hold state" in the joystick controller store, which makes it stop passing the commands forwards, allowing only for a confirmation-hold and a cancel button.

@ericjohnson97
Copy link
Contributor Author

I currently have a draft PR intended for merging, but after discussion with @patrickelectric , we concluded it requires some refinement, particularly regarding the current implementation of the "hold to confirm" feature. This feature presently occupies a joystick button slot, an outcome we aim to avoid. The solution we are working on involves unifying the "hold to confirm" and "slide to confirm" functionalities under a single library, distinguishing them solely by their confirmation method, either via joystick or UI.

I like the idea of hold to confirm and I also like the idea of unifying hold to confirm and slide to confirm. I think that makes a lot of sense. I think you all can close this pull request. I do still find it unwieldy to have to slide on button presses right now, but I can just keep these changes local then abandon them when you guy's elegant solution becomes available.

@rafaellehmkuhl
Copy link
Member

I like the idea of hold to confirm and I also like the idea of unifying hold to confirm and slide to confirm. I think that makes a lot of sense. I think you all can close this pull request. I do still find it unwieldy to have to slide on button presses right now, but I can just keep these changes local then abandon them when you guy's elegant solution becomes available.

To update on this: we discussed yesterday and decided to keep by default the old behavior of not asking for confirmation when the action is triggered on the joystick. We will also include a "hold to confirm" functionality, but this one will be opt-in (not activated by default).

Both will enter on the PR from @JoaoMario109 (#822), in a way we don't have to implement this function-wise (which would add a lot of unnecessary code to maintain).

@ericjohnson97
Copy link
Contributor Author

I like the idea of hold to confirm and I also like the idea of unifying hold to confirm and slide to confirm. I think that makes a lot of sense. I think you all can close this pull request. I do still find it unwieldy to have to slide on button presses right now, but I can just keep these changes local then abandon them when you guy's elegant solution becomes available.

To update on this: we discussed yesterday and decided to keep by default the old behavior of not asking for confirmation when the action is triggered on the joystick. We will also include a "hold to confirm" functionality, but this one will be opt-in (not activated by default).

Both will enter on the PR from @JoaoMario109 (#822), in a way we don't have to implement this function-wise (which would add a lot of unnecessary code to maintain).

That sounds like a good plan.

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.

None yet

4 participants