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

Doc outdated: Camera Protocol #298

Closed
mnumanuyar opened this issue Dec 9, 2020 · 9 comments · Fixed by #351
Closed

Doc outdated: Camera Protocol #298

mnumanuyar opened this issue Dec 9, 2020 · 9 comments · Fixed by #351

Comments

@mnumanuyar
Copy link
Contributor

MAV_CMD_REQUEST_CAMERA_SETTINGS message is depricated but is mentioned in doc.
Bug Page: Camera Protocol · MAVLink Developer Guide

@hamishwillee
Copy link
Collaborator

@mnumanuyar You're right and this is a known issue. The intention was to replace all the specific requestors with MAV_CMD_REQUEST_MESSAGE. However cameras need to support both, and right now I don't know if they will respond to that.

@dogmaphobic How do we push this forward? I want docs to be consistent. Can't GCS check for MAV_CMD_REQUEST_MESSAGE working and use old message as fallback?

@dogmaphobic
Copy link
Contributor

Not sure this will happen any time soon. The vast majority of existing systems do not support MAV_CMD_REQUEST_MESSAGE. It's not a simple change to add code in QGC to use MAV_CMD_REQUEST_MESSAGE and if no response is received, fall back to the old messages. With the existing command retry protocol, it would be around 10 seconds before the code would give up and switch to the old messages. For each one of them. Sending both at the same time would still be an issue as one of the commands would time out and there would need to have some special code to handle that exception. In all cases where this is used.

@hamishwillee
Copy link
Collaborator

@dogmaphobic The check for MAV_CMD_REQUEST_MESSAGE could be an on-connection request and cached. You could probably do it for just one request because if any work, they likely all will.

@dogmaphobic
Copy link
Contributor

if any work, they likely all will.

Not likely. You may have a flight controller who supports it and a camera and/or gimbal that don't. Or some other combination. Either way, you would need to have code to handle the test, the time out, and the switch to the old message. A direct switch is trivial if dropping support for existing cameras is acceptable though.

@mnumanuyar
Copy link
Contributor Author

@hamishwillee In anycase I think a note that explains the situtaion should be added somewhere to prevent confusion.

@hamishwillee
Copy link
Collaborator

Discussed this in Mavlink dev call. @LorenzMeier is going to discuss with @dogmaphobic to work out a way forward.

@mnumanuyar
Copy link
Contributor Author

thank you very much

@hamishwillee
Copy link
Collaborator

@julianoes Did you have a chance to talk this through with Jonas?

@julianoes
Copy link
Contributor

@hamishwillee ok I did discuss this with @JonasVautherin and here is what we suggest to do:

  1. Make pull requests to the places that we're aware of where MAV_CMD_REQUEST_MESSAGE is not yet supported but should be, so that the camera servers support both.
  2. Change the docs here stating that MAV_CMD_REQUEST_MESSAGE is to be preferred but for a transition period the old commands are still required.
  3. Wait a year.
  4. Update MAVSDK and QGC to use the new MAV_CMD_REQUEST_MESSAGE but fall back to old commands if there is no answer within a timeout.

As far as we know the one thing blocking us from moving quicker on this is the camera implementation of the Yuneec H520. I'm not aware of anything else out there that we can't influence to update.

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 a pull request may close this issue.

4 participants