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

Adding Slider to Adjust Altitude #721

Merged

Conversation

ericjohnson97
Copy link
Contributor

@ericjohnson97 ericjohnson97 commented Feb 5, 2024

closes #580

This slider component is designed to be similar to the slider in QGC for adjusting altitude. It is shown when an altitude change is commanded and is set when the user "slides to confirm"

2024-02-04.18-28-24.mp4

@rafaellehmkuhl
Copy link
Member

Pretty cool!
Could you add an alert (on the alert system) for when the command is accepted? It would be a good feedback for the user to know it worked (besides actually looking at the altitude, of course).

src/views/WidgetsView.vue Outdated Show resolved Hide resolved
src/components/AltitudeSlider.vue Show resolved Hide resolved
src/stores/mainVehicle.ts Outdated Show resolved Hide resolved
@ericjohnson97
Copy link
Contributor Author

@rafaellehmkuhl I didn't know that if I left a comment on a line of code it was not publicly viewable until I hit "submit review". I had made those comments last night, and didn't realize my mistake until now.

@ericjohnson97
Copy link
Contributor Author

Pretty cool! Could you add an alert (on the alert system) for when the command is accepted? It would be a good feedback for the user to know it worked (besides actually looking at the altitude, of course).

I think that could be a good addition. Do you think it would be best done based on the command accepted ack message? If so, then we could implement that as a generic alert as we had talked about in the past. similar to this #628 but for accepted.

Otherwise I could make an alert when the command is executed. Or I could make a special case to listen for the ack, but without the ack confirmation I won't truly know if it has been accepted.

@rafaellehmkuhl
Copy link
Member

Pretty cool! Could you add an alert (on the alert system) for when the command is accepted? It would be a good feedback for the user to know it worked (besides actually looking at the altitude, of course).

I think that could be a good addition. Do you think it would be best done based on the command accepted ack message? If so, then we could implement that as a generic alert as we had talked about in the past. similar to this #628 but for accepted.

Otherwise I could make an alert when the command is executed. Or I could make a special case to listen for the ack, but without the ack confirmation I won't truly know if it has been accepted.

You're right. Let's leave that for a separated PR.

@rafaellehmkuhl
Copy link
Member

@ericjohnson97 before opening the PR for review, could you rewrite the commits to follow more or less the current pattern on the repository commits (e.g.: letter case, way of writing, etc).

@ericjohnson97
Copy link
Contributor Author

@ericjohnson97 before opening the PR for review, could you rewrite the commits to follow more or less the current pattern on the repository commits (e.g.: letter case, way of writing, etc).

Of course. I wanted to make sure you were OK with the general implementation before I cleaned up my developmental commits.

@ericjohnson97 ericjohnson97 force-pushed the feature/altitude_slider branch from dd104b1 to b044fa3 Compare February 10, 2024 19:09
@ericjohnson97 ericjohnson97 marked this pull request as ready for review February 10, 2024 19:14
Copy link
Member

@rafaellehmkuhl rafaellehmkuhl left a comment

Choose a reason for hiding this comment

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

This looks good to me! Can we merge it @ericjohnson97?

@ericjohnson97
Copy link
Contributor Author

This looks good to me! Can we merge it @ericjohnson97?

It should be good to go!

@rafaellehmkuhl rafaellehmkuhl merged commit 2216d7b into bluerobotics:master Feb 14, 2024
7 checks passed
@rafaellehmkuhl rafaellehmkuhl added the docs-needed Change needs to be documented label Feb 21, 2024
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.

Altitude Slider
2 participants