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 vendor specs for cubepilot #1901

Merged
merged 1 commit into from
Oct 9, 2022

Conversation

bugobliterator
Copy link
Contributor

This PR adds mavlink messages used by Cubepilot hardware, and also reserves IDs for future use. This has already been merged in Ardupilot/mavlink fork.

@dagar
Copy link
Member

dagar commented Oct 8, 2022

What's wrong with VIDEO_STREAM_INFORMATION in common?http://mavlink.io/en/messages/common.html#VIDEO_STREAM_INFORMATION

@bugobliterator
Copy link
Contributor Author

What's wrong with VIDEO_STREAM_INFORMATION in common?http://mavlink.io/en/messages/common.html#VIDEO_STREAM_INFORMATION

We have been using the structure mentioned in the commit before existing VIDEO_STREAM_INFORMATION was realised. The structure works for us, we don't intend to change it in short term.

@dagar
Copy link
Member

dagar commented Oct 9, 2022

before existing VIDEO_STREAM_INFORMATION was realised

Before May 2017? #677

@bugobliterator
Copy link
Contributor Author

before existing VIDEO_STREAM_INFORMATION was realised

Before May 2017? #677

That is the form that we used. its changed since then.

@hamishwillee
Copy link
Collaborator

There are also messages in development for updating firmware - https://mavlink.io/en/messages/development.html#MAV_CMD_DO_UPGRADE

We strongly encourage you to use common messages; and if they aren't fit for your purpose to help us understand how we can improve them; it is better for everyone to use common messages because that eases integration.

That said, there is no obligation for you to do so. The only rule here is that the changes to dialect files are made by an authorized stakeholder, and that new files have the information needed for us to know who such an authorized person is. I've checked ArduPilot and this matches, so it is fine.

Thanks

@hamishwillee hamishwillee merged commit 33dde55 into mavlink:master Oct 9, 2022
@julianoes
Copy link
Collaborator

julianoes commented Oct 9, 2022

I was going to review this but I guess I'm late to the party.

I would appreciate if we don't merge things within a weekend next time.

@julianoes
Copy link
Collaborator

@bugobliterator We have been using the structure mentioned in the commit before existing VIDEO_STREAM_INFORMATION was realised. The structure works for us, we don't intend to change it in short term.

Any chance we could converge on this one in the future though? I'm coming from the perspective of the user who wants video streaming to "just work". If this message is used, then the video streaming protocol does not work for someone that uses their own built QGC or application based on MAVSDK.

The entire point of MAVLink is to standardize on these APIs to make integration more seamless.

@julianoes
Copy link
Collaborator

Anyway, I guess @hamishwillee is correct. You are allowed to do whatever you want, so I'll shut up now.

@hamishwillee
Copy link
Collaborator

Well, it's really pretty frustrating. Originally the theory was that common was a library that you should use IF you wanted the functionality covered. We kind of threw that away when we supported OllieW external dialect and then merged it with the others.

I'll add it to the agenda to the Dev Call. My take on this though is that if people want to self-harm it is hard to stop them.

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