-
Notifications
You must be signed in to change notification settings - Fork 18.6k
DroneCAN: support sending/receiving DroneCAN messages in lua #29310
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
Conversation
d4e5afe
to
ebfa1e5
Compare
ebfa1e5
to
60a1e4e
Compare
a3ada15
to
7096e3f
Compare
c1904d7
to
0bfefcd
Compare
I have to admit I don't understand what getting the internal libcanard semaphore accomplishes. Is it just to avoid deadlocks? I don't think that can happen here. |
0bfefcd
to
fa05e8a
Compare
Ah yes, I see that now. But why don't we just release This also applies to the destroy_all method, I don't see the danger in letting handlers continue to execute while the subscribers are destroyed. The canard lock will protect things. |
cbcd22b
to
fb330e8
Compare
bad480a
to
b38d9c7
Compare
@tpwrules I've removed the libcanard get_semaphore() and the per-handle semaphore, replacing it with get_sem_rx() in AP_DroneCAN |
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.
Happy to see the semaphores cleaned up. I think the per-handler semaphore would have worked fine, but using the DroneCAN one probably saves memory (at the cost of CPU and interference risk).
I definitely would want to see the battery tag changes pulled out in a separate PR.
d03255e
to
b5b36f4
Compare
b5b36f4
to
205eb34
Compare
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.
Scripting stuff looks good, just a small thing in the docs.
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.
Some error condition handling but overall looks good.
bfc73d4
to
2882056
Compare
2882056
to
42c8214
Compare
added get_semaphore() and allow for multiple handlers for an ID
allow for sending broadcast messages, subscribing to broadcast messages and sending requests
42c8214
to
43f4c27
Compare
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 little test example works well on Cube Orange. Great work! I also did some futzing with the REPL and could not immediately break the autopilot or my node.
Future expansion ideas:
- Autotest
- Ability to have autopilot itself/other scripts see script-sent messages
- DroneCAN server support (your idea)
- Priority handler/inhibit mechanism so a script can eat messages
- C++-assisted encoding and decoding
@tpwrules thanks for your help with this PR! |
This adds support for sending and receiving DroneCAN messages from lua scripts.
It includes an example script that subscribes to messages, sends requests and sends broadcast messages
Note that I have not done a lua compiler for DSDL, so manual message parsing is needed
This PR relies on this libcanard PR: dronecan/libcanard#74
and this DSDL update: dronecan/DSDL#67
Due to the simplification of libcanard this PR also saves about 4k of flash
should I move the GlobalTime broadcast to C++? Depends if other devices could use it