-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
uORB::Publication improvements and cleanup #14784
Conversation
dagar
commented
Apr 29, 2020
•
edited
Loading
edited
- common PublicationBase
- PublicationQueued types are now type aliases
- ORB_PRIO use type everywhere
- drivers (PX4Accelerometer/PX4Gyroscope/etc) advertise publications in constructor along with driver registration
b7556de
to
4b6e8ad
Compare
- common PublicationBase - PublicationQueued types are now type aliases - ORB_PRIO use type everywhere - drivers advertise on construction
4b6e8ad
to
fcf330f
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.
Is the ORB_PRIO
uint8_t -> int change noticeable?
{ | ||
if (_handle != nullptr) { | ||
// don't automatically unadvertise queued publications (eg vehicle_command) | ||
if (static_cast<DeviceNode *>(_handle)->get_queue_size() == 1) { |
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.
We need to double-check if that works as expected. No case specifically comes to mind, ulog_stream_ack
might be a candidate.
Overall we save a small amount with this PR (maybe a few hundred bytes), so it seems uORB::Publication using an ORB_ID class enum (uint8_t) has roughly made up for it. I didn't necessarily want to use enum ORB_PRIO everywhere, but I was a bit appalled when I saw the error in sf1xx. Longer term we might be able to make it an enum class or quite honestly the entire ORB priority concept might not really make sense to keep in this form. |
TODO: manually verify queued publication unadavertise behavior. |
I've stepped through this a few times and verified we're unadvertising regular Publications and leaving Queued. |
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.
Can you test that log streaming still works by restarting it several times?
Seems to be working. nsh> logger status
INFO [logger] Running in mode: all
INFO [logger] Number of subscriptions: 120 (3840 bytes)
INFO [logger] Full File Logging Running:
INFO [logger] Log file: /fs/microsd/log/sess006/log001.ulg
INFO [logger] Wrote 1.80 MiB (avg 14.51 KiB/s)
INFO [logger] Since last status: dropouts: 261 (max len: 0.677 s), max used buffer: 13009 / 14336 B
nsh> INFO [logger] Start mavlink log
nsh> logger status
INFO [logger] Running in mode: all
INFO [logger] Number of subscriptions: 120 (3840 bytes)
INFO [logger] Full File Logging Running:
INFO [logger] Log file: /fs/microsd/log/sess006/log001.ulg
INFO [logger] Wrote 2.70 MiB (avg 14.32 KiB/s)
INFO [logger] Since last status: dropouts: 1 (max len: 0.517 s), max used buffer: 12723 / 14336 B
INFO [logger] Mavlink Logging Running (Full log)
nsh> mavlink status
instance #0:
GCS heartbeat: 269194 us ago
mavlink chan: #0
type: USB CDC
flow control: OFF
rates:
tx: 79.366 kB/s
txerr: 1.515 kB/s
tx rate mult: 0.981
tx rate max: 800000 B/s
rx: 0.598 kB/s
ULog rate: 6.7% of max 70.0%
FTP enabled: YES, TX enabled: YES
mode: Config
MAVLink version: 2
transport protocol: serial (/dev/ttyACM0 @2000000)
ping statistics:
last: 6.84 ms
mean: 7.79 ms
max: 107.71 ms
min: 0.27 ms
dropped packets: 0
nsh> INFO [logger] Stop mavlink log
nsh> INFO [logger] Start mavlink log
nsh> logger status
INFO [logger] Running in mode: all
INFO [logger] Number of subscriptions: 120 (3840 bytes)
INFO [logger] Full File Logging Running:
INFO [logger] Log file: /fs/microsd/log/sess006/log001.ulg
INFO [logger] Wrote 3.65 MiB (avg 13.85 KiB/s)
INFO [logger] Since last status: dropouts: 141 (max len: 0.843 s), max used buffer: 0 / 14336 B
INFO [logger] Mavlink Logging Running (Full log)
nsh> INFO [logger] Stop mavlink log
nsh> logger status
INFO [logger] Running in mode: all
INFO [logger] Number of subscriptions: 120 (3840 bytes)
INFO [logger] Full File Logging Running:
INFO [logger] Log file: /fs/microsd/log/sess006/log001.ulg
INFO [logger] Wrote 3.87 MiB (avg 13.85 KiB/s)
INFO [logger] Since last status: dropouts: 51 (max len: 0.704 s), max used buffer: 0 / 14336 B
nsh> INFO [logger] Start mavlink log
nsh> INFO [logger] Stop mavlink log https://review.px4.io/plot_app?log=512007ef-4fee-4cd9-8594-d449cccad7f2 Side question, is it possible to capture postflight additional data when log streaming is stopped? |
Not easily, as stopping is requested externally. |
- create common uORB::PublicationBase - uORB::PublicationQueued types are now type aliases - ORB_PRIO use enum type everywhere to avoid accidental misuse - PX4Accelerometer/PX4Gyroscope/etc driver libs explicitly advertise on construction, unadvertise on destruction. This is a workaround for any potential issues that might appear when accel/gyro cdev and uORB indexing doesn't align.