Skip to content
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

samples: lora: receive: update SNR unit to dB #82451

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JordanYates
Copy link
Collaborator

Signal-to-noise ratio is a unitless quantity, so its unit is dB, not dBm (dBm is the power relative to 1 milliwatt). At the same time output the received payload using LOG_HEXDUMP_INF, instead of limiting the sample solely to string payloads.

Fixes #82414

Ideally this would also update the README sample output, but I no longer have access to any hardware with a LoRa modem to regenerate it.

@kartben
Copy link
Collaborator

kartben commented Dec 3, 2024

Ideally this would also update the README sample output, but I no longer have access to any hardware with a LoRa modem to regenerate it.

Ya, that can be confusing. I'll try to take care of this

martinjaeger
martinjaeger previously approved these changes Dec 3, 2024
Copy link
Collaborator

@kartben kartben left a comment

Choose a reason for hiding this comment

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

logging for sync receives, at L82, also needs updating

@@ -28,8 +28,8 @@ void lora_receive_cb(const struct device *dev, uint8_t *data, uint16_t size,
ARG_UNUSED(dev);
ARG_UNUSED(size);

LOG_INF("Received data: %s (RSSI:%ddBm, SNR:%ddBm)",
data, rssi, snr);
LOG_INF("LoRa RX RSSI: %ddBm, SNR: %ddB", rssi, snr);
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: also, please add a space between number and unit as per SI rules.

@kartben
Copy link
Collaborator

kartben commented Dec 3, 2024

@JordanYates

updated log output (with the spaces between numbers and units)

.. code-block:: console

   [00:00:00.235,000] <inf> lora_receive: Synchronous reception
   [00:00:00.956,000] <inf> lora_receive: LoRa RX RSSI: -60dBm, SNR: 7dB
   [00:00:00.956,000] <inf> lora_receive: LoRa RX payload
                                          68 65 6c 6c 6f 77 6f 72  6c 64                   |hellowor ld
   [00:00:02.249,000] <inf> lora_receive: LoRa RX RSSI: -57dBm, SNR: 9dB
   [00:00:02.249,000] <inf> lora_receive: LoRa RX payload
                                          68 65 6c 6c 6f 77 6f 72  6c 64                   |hellowor ld
   [00:00:03.541,000] <inf> lora_receive: LoRa RX RSSI: -57dBm, SNR: 9dB
   [00:00:03.541,000] <inf> lora_receive: LoRa RX payload
                                          68 65 6c 6c 6f 77 6f 72  6c 64                   |hellowor ld
   [00:00:04.834,000] <inf> lora_receive: LoRa RX RSSI: -55dBm, SNR: 9dB
   [00:00:04.834,000] <inf> lora_receive: LoRa RX payload
                                          68 65 6c 6c 6f 77 6f 72  6c 64                   |hellowor ld
   [00:00:04.834,000] <inf> lora_receive: Asynchronous reception
   [00:00:06.127,000] <inf> lora_receive: LoRa RX RSSI: -55dBm, SNR: 9dB
   [00:00:06.127,000] <inf> lora_receive: LoRa RX payload
                                          68 65 6c 6c 6f 77 6f 72  6c 64                   |hellowor ld
   [00:00:07.419,000] <inf> lora_receive: LoRa RX RSSI: -55dBm, SNR: 9dB
   [00:00:07.419,000] <inf> lora_receive: LoRa RX payload
                                          68 65 6c 6c 6f 77 6f 72  6c 64                   |hellowor ld
   [00:00:08.712,000] <inf> lora_receive: LoRa RX RSSI: -55dBm, SNR: 9dB
   [00:00:08.712,000] <inf> lora_receive: LoRa RX payload
                                          68 65 6c 6c 6f 77 6f 72  6c 64                   |hellowor ld
   [00:00:10.004,000] <inf> lora_receive: LoRa RX RSSI: -55dBm, SNR: 9dB
   [00:00:10.004,000] <inf> lora_receive: LoRa RX payload
                                          68 65 6c 6c 6f 77 6f 72  6c 64                   |hellowor ld
   [00:00:11.297,000] <inf> lora_receive: LoRa RX RSSI: -55dBm, SNR: 9dB
   [00:00:11.297,000] <inf> lora_receive: LoRa RX payload
                                          68 65 6c 6c 6f 77 6f 72  6c 64                   |hellowor ld
   [00:00:12.590,000] <inf> lora_receive: LoRa RX RSSI: -55dBm, SNR: 9dB
   [00:00:12.590,000] <inf> lora_receive: LoRa RX payload
                                          68 65 6c 6c 6f 77 6f 72  6c 64                   |hellowor ld
   [00:00:13.884,000] <inf> lora_receive: LoRa RX RSSI: -55dBm, SNR: 9dB
   [00:00:13.884,000] <inf> lora_receive: LoRa RX payload
                                          68 65 6c 6c 6f 77 6f 72  6c 64                   |hellowor ld
   [00:00:15.177,000] <inf> lora_receive: LoRa RX RSSI: -55dBm, SNR: 9dB
   [00:00:15.177,000] <inf> lora_receive: LoRa RX payload
                                          68 65 6c 6c 6f 77 6f 72  6c 64                   |hellowor ld
   [00:00:16.470,000] <inf> lora_receive: LoRa RX RSSI: -55dBm, SNR: 9dB
   [00:00:16.470,000] <inf> lora_receive: LoRa RX payload
                                          68 65 6c 6c 6f 77 6f 72  6c 64                   |hellowor ld
   [00:00:17.762,000] <inf> lora_receive: LoRa RX RSSI: -55dBm, SNR: 9dB
   [00:00:17.762,000] <inf> lora_receive: LoRa RX payload
                                          68 65 6c 6c 6f 77 6f 72  6c 64                   |hellowor ld
   [00:00:17.762,000] <inf> lora_receive: Stopping packet receptions

kartben
kartben previously approved these changes Dec 3, 2024
Copy link
Collaborator

@kartben kartben left a comment

Choose a reason for hiding this comment

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

virtually a +1 (and thanks!) but there is a compliance error (sorry if it's due to an incorrectly formatted request for change from my side)

Signal-to-noise ratio is a unitless quantity, so its unit is dB, not
dBm (dBm is the power relative to 1 milliwatt). At the same time output
the received payload using `LOG_HEXDUMP_INF`, instead of limiting the
sample solely to string payloads.

Fixes zephyrproject-rtos#82414

Signed-off-by: Jordan Yates <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SNR unit on LoRa Receive example
5 participants