Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Open Drone ID Live GNSS and Arm status prototype #21647
base: main
Are you sure you want to change the base?
Open Drone ID Live GNSS and Arm status prototype #21647
Changes from all commits
d52da5f
621431d
be02029
a33d48d
9ff0c4f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Why are we streaming OPEN_DRONE_ID_ARM_STATUS? Its inbound from the remoteID hardware. Should be used to indicate healthy/unhealthy for the remoteID.
I.e. an update to #21652
I'm probably missing something ...
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.
The Bluemark module requires the stream to work correctly. Without it, the module health checks are not satisfied and arming is prevented. Perhaps Bluemark can clarify on why this is.
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.
how much of this behaviour is defined in the spec vs left to the manufacturer to figure out?
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.
@tridge may know more about the how the spec has been implemented. The vendor may have just an abstraction layer on top of ODID core library for their specific hardware. Or they may use proprietary software and just satisfy the mavlink ODID definitions and don’t use ardu (or ODID core library)
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.
Yeah even in Ardupilot, the
ARM_STATUS
message isn't getting sent out, so I am puzzled as to why Bluemark module needs it 🤔https://github.com/ArduPilot/ardupilot/blob/5fa8b887a2df8a85822f84f41eec64bef5066895/libraries/AP_OpenDroneID/AP_OpenDroneID.cpp#L266-L326
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.
Well in the standard Remote ID regulation, you need to make sure the Remote ID information is correct and there are no failures. So if the Remote ID module detect an issue, it should report it back to the GCS.
Note there are open PRs that needs to be merged into master of ArduRemoteID: PR#101 and PR#102 that will improve these checks or its status message.
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.
@BluemarkInnovations I was going to sarcastically say where did you get that "requirement", but you're absolutely right - its in the MAVLink spec:
The vehicle shouldn't start if it doesn't have this so what the GCS does is irrelevant. Though this is needed for monitoring the precise cause of arming failure, unless the flight stack were to emit an event (and none is standardized).
@dirksavage88 What is interesting here is that the status would also have to be updated in LOCATION (that is what appears to happen if the HEARTBEAT disappears).
I'll see if I can tidy the spec: mavlink/mavlink-devguide#511
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.
FWIW I don't think this should be streamed by default. If there is no GPS then firing this off is a waste of bandwidth. perhaps it could be requested by GCS or enabled by a parameter.
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.
Late to the party as I only started looking into Remote ID yesterday, but I had the same initial reaction as @hamishwillee back in June.
In my mind it's more logical if you just set up mavlink forwarding so that the GCS can subscribe directly to the topic from the Remote ID hardware without going through the "useless" steps of repackaging the incoming message and publishing it as uORB only to subscribe to the uORB message to republish a mavlink message with basically the same content as the initial message.
Maybe I am too naive and miss something essential here?
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.
Yes, that would make sense, just forwarding it. However, for a module that talks DroneCAN or some other protocol, this might be required again.