Skip to content

AP_Logger, add dBm unit to log structure - #34320

Merged
peterbarker merged 1 commit into
ArduPilot:masterfrom
olliw42:owpr-dBmlogunit
Sep 8, 2026
Merged

AP_Logger, add dBm unit to log structure#34320
peterbarker merged 1 commit into
ArduPilot:masterfrom
olliw42:owpr-dBmlogunit

Conversation

@olliw42

@olliw42 olliw42 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Summary

I kindly ask to add a unit specifier for dBm to the log structure. It may not be used inside ArduPilot code, but Lua scripts are allowed to define their own datalog messages, where it can be highly useful. The specific application is mLRS, which provides an ArduPilot Lua script which allows us to log substantial link performance and link information metrics, among which are lots of "dBm types", such as rssi, snr and power.

Classification & Testing (check all that apply and add your own)

  • [ x] Checked by a human programmer
  • Non-functional change
  • No-binary change
  • Infrastructure change (e.g. unit tests, helper scripts)
  • Automated test(s) verify changes (e.g. unit test, autotest)
  • Tested manually, description below (e.g. SITL)
  • Tested on hardware
  • Logs attached
  • Logs available on request

Description

@olliw42
olliw42 force-pushed the owpr-dBmlogunit branch 2 times, most recently from 93e9a26 to c82caa8 Compare September 7, 2026 17:31
@olliw42

olliw42 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

I am sorry for the force pushes, I tried to change the commit message to start with AP_Logger: xxxx, but my efforst failed. Don't knwo what to do else.

@rmackay9

rmackay9 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

While we don't normally have code in AP that isn't used, my guess is we can make an exception because of the lua script mentioned. I think the next question is just about whether "R" is the right letter to use. It's a bit arbitrary but we normally pick a letter that's related somehow.

@rmackay9 rmackay9 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After a quick chat with @timtuxworth "R" for "rssi" seems OK to me. Other letters like "d", "D", "b", "B" and "m" are already taken. "M" is free but that doesn't seem any better.

@timtuxworth timtuxworth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a good idea to me - I see "R" for RSSI makes sense.

@olliw42

olliw42 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

my line of thoughts was, 'd', 'D' (for dBm), 'p', 'P' (for power) are taken, so maybe 'R' alluding to RF or RADIO or ... well, rssi

we don't normally have code in AP that isn't used

I know that well and perfectly understand that.
But Lua scripts exist (and the mentioned one is not the first and only using logger). An alternative could be to provide a lua means and a dataflash log means (new FMT ??) to record user supplied units. Would be more generic, also would help viewers, and avoid that, but is way more complicated.

@rmackay9

rmackay9 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Hi @olliw42,

Could you rebase on master? We don't accept merge commits into AP because we find it makes backporting more difficult - we prefer a linear history.

@peterbarker

Copy link
Copy Markdown
Contributor

When choosing letters, try to stay away from SI unit shortenings.

R looks pretty safe.

@olliw42

olliw42 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

ah ... it offered me a button to update to main ... sorry for clicking. Corrected.

@peterbarker
peterbarker merged commit 7cf7aa9 into ArduPilot:master Sep 8, 2026
101 checks passed
@olliw42

olliw42 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

MANY THX!
this was easy and smooth

will this appear in AP 4.8, or may it be backported, e.g to 4.7.2?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants