samples: bluetooth: central_uart: status LEDs for nRF54LM20 Dongle#27722
Conversation
CI InformationTo view the history of this post, click the 'edited' button above Inputs:Sources:sdk-nrf: PR head: 1213d70c67e8285a17f996c773f1a339039dd2f9 more detailssdk-nrf:
Github labels
List of changed files detected by CI (7)Outputs:ToolchainVersion: 911f4c5c26 Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped;
|
115e6df to
58b36d5
Compare
|
You can find the documentation preview for this PR here. Preview links for modified nRF Connect SDK documents: https://ncsbmdoc.z6.web.core.windows.net/ncs/PR-27722/nrf/releases_and_maturity/releases/release-notes-changelog.html |
ktaborowski
left a comment
There was a problem hiding this comment.
looks good, minor comment below
| */ | ||
| static const struct gpio_dt_spec log_led = | ||
| GPIO_DT_SPEC_GET(DT_CHOSEN(nordic_central_uart_log_led), gpios); | ||
| static int log_led_blink_remaining; |
There was a problem hiding this comment.
minor: Type inconsistency between this and ble_led_blink_remaining (is uint8_t)
| k_timer_stop(timer); | ||
| gpio_pin_set_dt(&ble_led, 1); |
There was a problem hiding this comment.
potential race condition - if leds_set_ble_connected(false) happens between those two, the led stays on, despite the ble is disconnected. I don't know what is the probability of such event (probably very low).
Writing this way it will fix:
gpio_pin_set_dt(&ble_led, ble_connected ? 1 : 0);
There was a problem hiding this comment.
That's why I stop the timer in leds_set_ble_connected so I think there's no such a risk, right? Timer handler is executed from ISR, so it cannot be preempted by a thread.
| void leds_init(void) | ||
| { | ||
| if (!gpio_is_ready_dt(&ble_led)) { | ||
| return; |
There was a problem hiding this comment.
There is no log in this module, also all functions in leds.h are void. Please consider some logging on fails.
Readme says nRF54LM20 Dongle The sample uses the CDC ACM interface with index 0 for debug logging
There was a problem hiding this comment.
Would you like me to check all error codes, such as k_timer_stop, k_timer_start and such or are you interested in specific ones?
There was a problem hiding this comment.
Added some logs and asserts here and there.
58b36d5 to
d6f865c
Compare
On nRF54LM20 Dongle, let: - green LED indicates BLE connection state and traffic - blue LED indicate logging activity. Signed-off-by: Damian Krolik <damian.krolik@nordicsemi.no>
d6f865c to
1213d70
Compare
|
@nordicjm Please review |
On nRF54LM20 Dongle, let: