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

Do not set GCS mode as default #2125

Conversation

ThomasDebrunner
Copy link
Contributor

Setting the GCS mode as default has hidden side-effects. Most notably that the connection is considered a valid datalink, and thus affects datalink failsafes in Autopilots.

When used on-board, this is an unwanted side-effect. While you can configure this, the default is set to the "less-safe" option, which could leave a careless user in the situation where datalink failsafes get disabled unintentionally.

@JonasVautherin
Copy link
Collaborator

JonasVautherin commented Aug 29, 2023

I am probably a bit biased because I always saw MAVSDK primarily as an API for ground stations, which can also be used on the other side (be it a companion or an autopilot). To me it feels like whoever uses MAVSDK for something other than a ground station should know that in the MAVLink world, they should care about their sysid/compid (I don't think MAVLink gives us the luxury to be careless users 🙈). MAVSDK provides a way to set them, and it changes the type accordingly (you can say that you want to be an autopilot or a companion computer, for instance).

In any case, it feels weird to set the sysid/compid to values reserved for a ground station (which is the default), and at the same time to not set the type to "ground station" 🤔. I agree that the specs are not completely clear (some sysid/compids seem to be attributed to some types, which probably should never have been the case), but that is what we have.

On top of that, it is a breaking change for all the current users of MAVSDK as a ground station... Maybe that could go in MAVSDK v2?

@julianoes what do you think?

@julianoes
Copy link
Collaborator

I can see the drawbacks of having a default that causes surprises, and I would always try to err on the side of safety/caution.

However, I'm not convinced changing it to Custom as custom implies something is customized, and not default/standard. My plan was to overhaul the configuration and component type stuff to sync the two concepts and share the enums, etc. It seems like it would also be a good idea to make the selection required in the constructor, to avoid defaults that don't work. This would be a change we could do with v2.

@julianoes
Copy link
Collaborator

@ThomasDebrunner what about something like this?

Mavsdk mavsdk{Configuration{Camera}};
CameraServer camera_server{mavsdk.default_component()}; // using MAV_COMP_ID_CAMERA1
WinchServer winch_server{mavsdk.component_by_id(MAV_COMP_ID_WINCH)}; // or whatever, if additional componens within a mavsdk server are required.

@julianoes
Copy link
Collaborator

Done in #2169.

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