-
Notifications
You must be signed in to change notification settings - Fork 94
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
remote_id: open drone id plugin interface #327
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your PR @persuader72. I'm appalled by the OpenDrone ID MAVLink messages, now that I look at them in detail but a well, too late now, I suppose.
However, we have to figure out which things we want to expose raw and what we can hide and make easier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few nitpicks 😇.
With the "ArmStatus" stream i think that the feature set is completed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks persuading 😄
enum VerAcc { | ||
VER_ACC_UNKNOWN = 0; // The vertical accuracy is unknown. | ||
VER_ACC_METER_150 = 1; // The vertical accuracy is smaller than 150 meter. | ||
VER_ACC_METER_45 = 2; // The vertical accuracy is smaller than 45 meter. | ||
VER_ACC_METER_25 = 3; // The vertical accuracy is smaller than 25 meter. | ||
VER_ACC_METER_10 = 4; // The vertical accuracy is smaller than 10 meter. | ||
VER_ACC_METER_3 = 5; // The vertical accuracy is smaller than 3 meter. | ||
VER_ACC_METER_1 = 6; // The vertical accuracy is smaller than 1 meter. | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still puzzles my why they wouldn't just use a uint8
to signal meter accuracy... but anyway, I think I can live with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the reasons are historic. In some radio broadcasting ID protocols (like AIS or ADSB) the accuracy is an enumerative type or is rounded to less than 8 bits to save space (then save bandwidth)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, fair enough.
@julianoes, It is better to rewrite the history of this PR in one single commit? |
@persuader72 Feel free to rewrite the history and force push. If you don't, then I'll just squash-merge it. |
858de5e
to
415ab27
Compare
Sorry, this disappeared off my radar. I will review again ASAP! |
@julianoes @persuader72 Is it possible to merge this? |
Sorry yes, I need to work through this. I have it planned as soon as v3 is done (see mavlink/MAVSDK#2316). |
Proto file for the remote_id protocol.
This proto file is need by the new remote_id MAVSDK plugin.
The goal of the plugin to use the companion computer to work with open drone id compatible hardware like this even is the autopilot is not compatible with such protocol.