Change register_periodic_telemetry() to use msg ID and support other protocols (mavlink) #1448
Conversation
in the telemetry xml file a type can be specified for the process (default is "pprz"). You can also set "mavlink" and use mavlink messages, which will be sent as specified if the mavlink module is loaded and that mavlink message is registered there.
|
So I would vote for this solution, so basically dropping support for registering alias messages and requiring the messages to match instead. @gautierhattenberger @martinmm @dewagter what do you think? |
| }; | ||
|
|
||
| /** Register a telemetry callback function. | ||
| * empty implementation is provided if PERIODIC_TELEMETRY is not set or set to FALSE | ||
| * @param _pt periodic telemetry structure to register | ||
| * @param _msg message name (string) as defined in telemetry xml file | ||
| * @param _msgn message id/number (use DL_<message_name> define) |
There was a problem hiding this comment.
should be PPRZ_MSG_ID_
|
I'm fine with this one |
flixr
force-pushed
the
register_with_msg_id
branch
from
4662823
to
9f4c58c
Compare
- if a mavlink process is specified in the telemetry file, use that, otherwise fall back to fixed message periods - print warning if telemety file has no mavlink process - fix code style
|
@dewagter @LodewijkSikkel also changed the mavlink module to send messages as specified in the telemetry xml file (if no mavlink process in telemetry file it will fall back to hardcoded periods). |
|
still fits into the Naze32 if the compressed sine is used, guess the overall size increased over the last 5 months; starts and looks ok |
Change register_periodic_telemetry() to use msg ID and support other protocols (mavlink) - Change register_periodic_telemetry() to use msg ID (PPRZ_MSG_ID_x or MAVLINK_MSG_ID_x) - possibility to use register_periodic_telemetry for mavlink and others. - in the telemetry xml file a type can be specified for the process (default is "pprz"). - You can also set "mavlink" and use mavlink messages, which will be sent as specified if the mavlink module is loaded and that mavlink message is registered there.
| // send periodic mavlink messages as defined in the Mavlink process of the telemetry xml file | ||
| // transport and device not used here yet... | ||
| periodic_telemetry_send_Mavlink(&mavlink_telemetry, NULL, NULL); | ||
| #else |
There was a problem hiding this comment.
I think this RunOnceEvery was needed before because there was no other option, but now that you have the telemetry file which is much more flexible, I would argue that this RunOnceEvery can be completely removed.
There was a problem hiding this comment.
I kept the RunOnceEvery for now so it will still work when you have a telemetry file without the mavlink process... just wanted to avoid breaking existing stuff...
But I wouldn't mind removing those (except for the send_params)
|
This looks great and is a milestone in the support of multiple protocols. Not really about this pull request: but is there no better technical solution than to loop through the list to match ID's on every message? Now that it are uint8_t, can't we have a list of 256 function pointers or is RAM more valuable than a few hundred machine codes per iteration? |
|
Of course we could have a list of 256 function pointers, but IMHO this is wasting space. |
Alternative to #1442:
This requires an extra unit8_t id field in the struct (so if you have 20 different messages in the telmetry file, this would add an extra 20bytes compared to #1442).
At runtime it will loop over the periodic messages until it finds the matching id (compared to already knowing the index).
Also this really makes it impossible to register "meta/alias" messages... but I would argue that even in #1442 this is not nice since you need to provide the #ifndefs in the code...