samples: bluetooth: silabs: Bluetooth blinky by silabs-vifisch · Pull Request #142 · SiliconLabsSoftware/zephyr-silabs

silabs-vifisch · 2026-01-08T15:30:01Z

Similar to Silabs' SOC Blinky sample, implements two GATT characteristics.
One characteristic controls the state of the LED via write operations from
a GATT client. Second characteristic sends notifications to subscribed
clients when the button state changes on the board.
Minor changes have been made in README.rst, samples.yaml CMakeList.txt
Some coding style and global variable issues have been fixed.

Johan Hedberg (jhedberg) · 2026-01-19T12:59:29Z

silabs-vifisch could you please fix the commit message so it makese sense. Now it seems to describe some small detail between two version of the commit, which is irrelevant once this gets merged. You can find extensive guidelines on writing good commit messages here: https://docs.zephyrproject.org/latest/contribute/guidelines.html#commit-message-guidelines

Jérôme Pouiller (jerome-pouiller) · 2026-01-26T16:40:12Z

+#include <zephyr/bluetooth/gatt.h>
+#include <zephyr/bluetooth/services/bas.h>
+
+static struct bt_conn *current_conn;


Avoid global variable (static const are accepted).

The connection object comes from a call-back function that needs to be from project wide it is more convenient to use a global variable current_conn. This is the reason I kept it.

bt_conn is an argument of the relevant callbacks (I believe you can use BT_GATT_CCC_MANAGED() instead of BT_GATT_CCC()).

At the end, I believe you won't need this variable.

bt_conn is used in two custom callback where the system doesn't give it back as an argument.
I'm afraid BT_GATT_CCC_MANAGED() won't help.

Jérôme Pouiller (jerome-pouiller) · 2026-01-26T16:40:37Z

@@ -0,0 +1,447 @@
+/* main.c - Application main entry point */


What is the purpose of a such trivial comment?

Removed! Actually it was originally in the base project.

Jérôme Pouiller (jerome-pouiller) · 2026-01-26T16:41:03Z

+      - qemu_cortex_m3
+    tags:
+      - bluetooth
+      - LED


Jérôme Pouiller (jerome-pouiller) · 2026-01-26T16:41:29Z

@@ -0,0 +1,14 @@
+sample:
+  name: Bluetooth Blinky
+  description: Similar to Silabs SOC Blinky


Please provide a true description.

Jérôme Pouiller (jerome-pouiller) · 2026-01-26T16:43:07Z

+    harness: bluetooth
+    integration_platforms:
+      - frdm_k64f
+      - qemu_cortex_m3


Does it make sense?

Jérôme Pouiller (jerome-pouiller) · 2026-01-26T16:55:54Z

+{
+	if (state > 0) {
+		led_value = 1;
+		led_report_value = 1;


Can't you replace these variable by gpio_pin_get() (single source of trust)?

As mentioned the are GATT entries they must hold valid states.

I don't get you. What prevent you to read the actual state in the callbacks?

As led_value is a GATT entry being single source of truth not the led.pin.

Jérôme Pouiller (jerome-pouiller) · 2026-01-26T16:57:19Z

+		printk("failed.\n");
+		return -EIO;
+	}
+	printk("done.\n");


Do not log success (you can log errors).

Jérôme Pouiller (jerome-pouiller) · 2026-01-26T16:58:28Z

+								uint32_t pins)
+{
+	/* Simple: toggle LED using current state */
+	set_led_state(!led_value);


gpio_pin_toggle_dt()?

Actually here the led state (in the GATT table) is set.

Jérôme Pouiller (jerome-pouiller) · 2026-01-26T16:59:43Z

+
+/* BT_LE_ADV_CONN_FAST_1 */
+
+#if !defined(CONFIG_BT_EXT_ADV)


Consider use of if (IS_ENABLED(CONFIG_BT_EXT_ADV)). I believe it should compile.

Pls. see comment above!

which comment?

Right upper comment, this is inherited from the previous project, it is not used here, so maybe it can be completely removed.

Jérôme Pouiller (jerome-pouiller) · 2026-06-18T12:16:38Z

+BT_GATT_SERVICE_DEFINE(led_svc,
+	BT_GATT_PRIMARY_SERVICE(&led_service_uuid),
+	BT_GATT_CHARACTERISTIC(&led_char_uuid.uuid,
+					BT_GATT_CHRC_READ | BT_GATT_CHRC_WRITE,


It is still no correct. BT_GATT_CHRC_READ should be aligned with the opening parenthesis of BT_GATT_CHARACTERISTIC(:

BT_GATT_SERVICE_DEFINE(led_svc, BT_GATT_PRIMARY_SERVICE(&led_service_uuid), BT_GATT_CHARACTERISTIC(&led_char_uuid.uuid, BT_GATT_CHRC_READ | BT_GATT_CHRC_WRITE, BT_GATT_PERM_READ | BT_GATT_PERM_WRITE, btapp_read_led, btapp_write_led, &led_value), BT_GATT_CHARACTERISTIC(&led_report_char_uuid.uuid, BT_GATT_CHRC_NOTIFY, BT_GATT_PERM_READ | BT_GATT_PERM_WRITE, NULL, NULL, &led_report_value), BT_GATT_CCC(btapp_led_report_ccc_cfg_changed, BT_GATT_PERM_READ | BT_GATT_PERM_WRITE) );

(Note it makes sense to have an exception on this rule for the arguments of BT_GATT_SERVICE_DEFINE())

Don't forgot to apply this rule to the other functions calls/declarations.

Jérôme Pouiller (jerome-pouiller) · 2026-06-18T12:35:42Z

+};
+
+static uint8_t led_value = 1;
+static uint8_t led_report_value = 1;


Unfortunately, it is difficult to track these variables (I have to admit after 10min I have still not understand what they do).

I believe You can simply get rid of led_value and led_report_value and only rely on gpio_pin_get(led.port, led.pin) and gpio_pin_set(led.port, led.pin, x) (so you will also comply with the rule of single source of trust).

Jérôme Pouiller (jerome-pouiller) · 2026-06-18T12:39:05Z

+		printk("Connected\n");
+
+		current_conn = conn;
+		(void)atomic_set_bit(state, STATE_CONNECTED);


it may be more states, and it is used project-wide that is why I kept it in its global form.

For now, you don't need it (YAGNI rule).

Jérôme Pouiller (jerome-pouiller) · 2026-06-18T12:43:37Z

+#include <zephyr/bluetooth/gatt.h>
+#include <zephyr/bluetooth/services/bas.h>
+
+static struct bt_conn *current_conn;


bt_conn is an argument of the relevant callbacks (I believe you can use BT_GATT_CCC_MANAGED() instead of BT_GATT_CCC()).

At the end, I believe you won't need this variable.

Jérôme Pouiller (jerome-pouiller) · 2026-06-18T12:49:15Z

+
+#if defined(CONFIG_GPIO)
+
+#if DT_NODE_HAS_STATUS_OKAY(LED0_NODE)


Does it make sense to be able to compile "blinky" without a led?

Jérôme Pouiller (jerome-pouiller) · 2026-06-18T13:04:22Z

+
+	/* Implement notification. */
+	while (1) {
+		k_sleep(K_SECONDS(1));


RTOS have been designed to avoid such polling loops (not realtime efficient, not scalable, not power efficient). Even if it not a big deal for this example, I know the customer tends to reuse the samples for their real products.

Maybe you could manage the event directly in the callbacks. If you can't, wake up the main loop using events or semaphore (see zpoll()).

The final customer will rely on an rely on the hardware (the ADC I assume) to get the change on the battery level. You can get something close with k_timer_start().

Jérôme Pouiller (jerome-pouiller) · 2026-06-18T13:06:47Z

+			printk("Successfully Connected\n");
+
+		} else if (atomic_test_and_clear_bit(state, STATE_DISCONNECTED)) {
+#if !defined(CONFIG_BT_EXT_ADV)


Can't you replace this #ifdef with if (IS_ENABLED(CONFIG_BT_EXT_ADV)) {?

Jérôme Pouiller (jerome-pouiller) · 2026-06-18T13:12:07Z

+
+/* BT_LE_ADV_CONN_FAST_1 */
+
+#if !defined(CONFIG_BT_EXT_ADV)


which comment?

Jérôme Pouiller (jerome-pouiller) · 2026-06-18T13:15:10Z

+{
+	if (state > 0) {
+		led_value = 1;
+		led_report_value = 1;


I don't get you. What prevent you to read the actual state in the callbacks?

Similar to Silabs' SOC Blinky sample, implements two GATT characteristics. One characteristic controls the state of the LED via write operations from a GATT client. Second characteristic sends notifications to subscribed clients when the button state changes on the board. Minor changes have been made in README.rst, samples.yaml CMakeList.txt Some coding style and global variable issues have been fixed. Signed-off-by: Viktor Fisch <viktor.fisch@silabs.com>

Major changes in main.c includes introduction of signal usage instead of state bits and battery level notification is moved to a scheduled task. Signed-off-by: Viktor Fisch <viktor.fisch@silabs.com>

silabs-vifisch requested a review from a team as a code owner January 8, 2026 15:30

silabs-vifisch force-pushed the bt_blinky branch from 132b991 to 8d87754 Compare January 19, 2026 12:26

silabs-vifisch force-pushed the bt_blinky branch from 8d87754 to a92b59a Compare January 19, 2026 14:51

Jérôme Pouiller (jerome-pouiller) reviewed Jan 26, 2026

View reviewed changes

silabs-vifisch force-pushed the bt_blinky branch from 7601499 to 1e5e1b4 Compare February 9, 2026 14:00

silabs-vifisch requested a review from Jérôme Pouiller (jerome-pouiller) February 17, 2026 09:15

Johan Hedberg (jhedberg) added DNM (manifest) and removed DNM (manifest) labels Feb 23, 2026

Jérôme Pouiller (jerome-pouiller) requested changes Jun 18, 2026

View reviewed changes

silabs-vifisch force-pushed the bt_blinky branch 2 times, most recently from e814648 to 1d3debe Compare July 7, 2026 10:15

samples: bluetooth: silabs: Bluetooth blinky

b699fac

Major changes in main.c includes introduction of signal usage instead of state bits and battery level notification is moved to a scheduled task. Signed-off-by: Viktor Fisch <viktor.fisch@silabs.com>

silabs-vifisch force-pushed the bt_blinky branch from 1d3debe to b699fac Compare July 7, 2026 10:35

		@@ -0,0 +1,447 @@
		/* main.c - Application main entry point */


		#if defined(CONFIG_GPIO)

		#if DT_NODE_HAS_STATUS_OKAY(LED0_NODE)

Uh oh!

Conversation

silabs-vifisch commented Jan 8, 2026 • edited by jhedberg Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Johan Hedberg (jhedberg) commented Jan 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

silabs-vifisch Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

silabs-vifisch commented Jan 8, 2026 •

edited by jhedberg

Loading

silabs-vifisch Feb 2, 2026 •

edited

Loading