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

Add camera configuration UI #2857

Merged
merged 4 commits into from
Sep 24, 2024

Conversation

Williangalvani
Copy link
Member

@Williangalvani Williangalvani commented Aug 9, 2024

No description provided.

@Williangalvani Williangalvani force-pushed the camera_stuff branch 2 times, most recently from 57afd0c to b951958 Compare August 27, 2024 15:44
Copy link
Member

@patrickelectric patrickelectric left a comment

Choose a reason for hiding this comment

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

@ES-Alexander (Mr. Elioto), can you put some of you magic touch in this range image ?

@ES-Alexander
Copy link
Collaborator

@patrickelectric these were my original design suggestions:

image

I expect @Williangalvani is starting out with a simpler interface, although given this PR is still a draft it's quite possibly still a WIP.

We did have some discussions about the diagram for this being much more dynamic than the others (e.g. adjusting the diagram based on the specified values as well as the autopilot telemetry for the current orientation and mount tilt angles), which does also make it trickier to have hand-drawn aspects for things like the arrows, if we want them to have variable lengths depending on the angles being covered...

There was also some discussion around a 3D interface, especially if we want to visualise the field of view:

@Williangalvani
Copy link
Member Author

@ES-Alexander that is correct. I decided to start smaller as this will cover 90% of what we need. Just to avoid sinking too much time into it. I also had misunderstood how some of those parameters work, which is why they are gone.

@Williangalvani Williangalvani marked this pull request as ready for review August 29, 2024 20:57
Copy link
Member

@patrickelectric patrickelectric left a comment

Choose a reason for hiding this comment

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

Is 0 degrees expected here ?

image

@patrickelectric
Copy link
Member

ping @Williangalvani

@eldinmiller
Copy link

I second that it would be useful to have a "center" or "home", it may not be the center of the two values. For example, you may want to have less range of motion up than down, but still center straight ahead.

@Williangalvani
Copy link
Member Author

I second that it would be useful to have a "center" or "home", it may not be the center of the two values. For example, you may want to have less range of motion up than down, but still center straight ahead.

hmmm interesting. that would require a small change on ardusub, but I think it is worth it

@Williangalvani Williangalvani force-pushed the camera_stuff branch 3 times, most recently from fc5b6c7 to c41f6e8 Compare September 17, 2024 18:06
@Williangalvani
Copy link
Member Author

Updated to include most of the functionality I originally intended to have.
The downside is that this now requires Sub >= 4.5
Screenshot 2024-09-17 at 15 08 51

@patrickelectric patrickelectric merged commit 55e8e6d into bluerobotics:master Sep 24, 2024
6 checks passed
@patrickelectric patrickelectric deleted the camera_stuff branch September 24, 2024 01:44
@ES-Alexander ES-Alexander added the docs-needed Change needs to be documented label Oct 7, 2024
@ES-Alexander ES-Alexander added docs-complete Change documentation has been completed and removed docs-needed Change needs to be documented labels Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-complete Change documentation has been completed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants