fix: BLE advertising/TX-power/LED/button parity polish

.github/workflows/main.yaml

@@ -0,0 +1,41 @@
+name: PlatformIO CI
+
+on: [push, pull_request]
+
+jobs:
+  build:
+    runs-on: ubuntu-latest
+
+    strategy:
+      fail-fast: false
+      matrix:
+        environment:
+          - seeed-xiao-nrf54l15
+          - seeed-xiao-nrf54lm20a
+
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/cache@v4
+        with:
+          path: |
+            ~/.cache/pip
+            ~/.platformio/.cache
+          key: ${{ runner.os }}-pio-${{ matrix.environment }}
+      - uses: actions/setup-python@v5
+        with:
+          python-version: '3.11'
+      - name: Install PlatformIO Core
+        run: pip install --upgrade platformio
+
+      - name: Build firmware
+        run: |
+          platformio run --environment ${{ matrix.environment }}
+
+      - name: Upload firmware artifact
+        uses: actions/upload-artifact@v4
+        with:
+          name: firmware-${{ matrix.environment }}
+          path: |
+            .pio/build/${{ matrix.environment }}/firmware.hex
+            .pio/build/${{ matrix.environment }}/zephyr/zephyr.hex
+          if-no-files-found: error

platformio.ini

@@ -13,7 +13,6 @@ build_flags =
   -DOD_APP_VERSION=0x0100
   -DOPENDISPLAY_BUILD_ID=\"nrf54\"
   -DOPENDISPLAY_ZLIB_USE_HEAP_WINDOW=0
-  -D__ARM_FEATURE_DSP=1
 
 [env:seeed-xiao-nrf54l15]
 platform = https://github.com/Seeed-Studio/platform-seeedboards.git
@@ -30,3 +29,7 @@ board = seeed-xiao-nrf54lm20a
 build_flags =
   ${env.build_flags}
   -DNRF54_BOARD_LM20
+; CMSIS 6 (this env's framework-zephyr) gates DSP wrappers on __ARM_FEATURE_DSP,
+; but g++ never declares the __sxtb16/__sxtab16 ACLE builtins they call, so every
+; C++ TU including a Zephyr header fails. Nothing here uses DSP intrinsics.
+  -U__ARM_FEATURE_DSP

src/nrf54_gpio.c

@@ -70,6 +70,68 @@ void nrf54_gpio_configure_input(uint8_t cfg, bool pull_up, bool pull_down)
 	(void)gpio_pin_configure(gpio_dev(port), pin, flags);
 }
 
+/* One slot per interrupt-enabled pin. Each slot owns its gpio_callback so the
+ * Zephyr trampoline can recover the registered handler via CONTAINER_OF. */
+#define NRF54_GPIO_IRQ_MAX 8
+
+struct nrf54_gpio_irq_slot {
+	struct gpio_callback cb;
+	nrf54_gpio_irq_handler_t handler;
+	bool used;
+};
+
+static struct nrf54_gpio_irq_slot s_irq_slots[NRF54_GPIO_IRQ_MAX];
+
+static void nrf54_gpio_irq_trampoline(const struct device *dev, struct gpio_callback *cb,
+				      uint32_t pins)
+{
+	ARG_UNUSED(dev);
+	ARG_UNUSED(pins);
+	struct nrf54_gpio_irq_slot *slot =
+		CONTAINER_OF(cb, struct nrf54_gpio_irq_slot, cb);
+
+	if (slot->handler != NULL) {
+		slot->handler();
+	}
+}
+
+int nrf54_gpio_configure_interrupt(uint8_t cfg, nrf54_gpio_irq_handler_t handler)
+{
+	uint8_t port;
+	uint8_t pin;
+	const struct device *dev;
+	struct nrf54_gpio_irq_slot *slot = NULL;
+	int err;
+
+	if (handler == NULL || !nrf54_pin_decode(cfg, &port, &pin)) {
+		return -1;
+	}
+	dev = gpio_dev(port);
+	for (unsigned i = 0; i < NRF54_GPIO_IRQ_MAX; i++) {
+		if (!s_irq_slots[i].used) {
+			slot = &s_irq_slots[i];
+			break;
+		}
+	}
+	if (slot == NULL) {
+		return -1;
+	}
+	slot->handler = handler;
+	slot->used = true;
+	gpio_init_callback(&slot->cb, nrf54_gpio_irq_trampoline, BIT(pin));
+	err = gpio_add_callback(dev, &slot->cb);
+	if (err != 0) {
+		slot->used = false;
+		return err;
+	}
+	err = gpio_pin_interrupt_configure(dev, pin, GPIO_INT_EDGE_BOTH);
+	if (err != 0) {
+		(void)gpio_remove_callback(dev, &slot->cb);
+		slot->used = false;
+	}
+	return err;
+}
+
 void nrf54_gpio_write(uint8_t cfg, bool level_high)
 {
 	uint8_t port;

src/nrf54_gpio.h

@@ -10,9 +10,17 @@ extern "C" {
 
 #define NRF54_GPIO_PIN_UNUSED 0xFFu
 
+/* Simple parameterless callback invoked from GPIO interrupt (ISR) context. The
+ * handler MUST NOT do I2C/BLE/blocking work; it should only set a flag that the
+ * main loop consumes. */
+typedef void (*nrf54_gpio_irq_handler_t)(void);
+
 bool nrf54_pin_decode(uint8_t cfg, uint8_t *port_out, uint8_t *pin_out);
 void nrf54_gpio_configure_output(uint8_t cfg, bool initial_high);
 void nrf54_gpio_configure_input(uint8_t cfg, bool pull_up, bool pull_down);
+/* Enable a both-edges GPIO interrupt on the pin, invoking handler in ISR
+ * context. Returns 0 on success, negative on error. */
+int nrf54_gpio_configure_interrupt(uint8_t cfg, nrf54_gpio_irq_handler_t handler);
 void nrf54_gpio_write(uint8_t cfg, bool level_high);
 int nrf54_gpio_read(uint8_t cfg);
 void nrf54_gpio_park(uint8_t cfg);

src/opendisplay_ble.c

@@ -14,7 +14,9 @@
 #include <zephyr/bluetooth/gatt.h>
 #include <zephyr/bluetooth/gap.h>
 #include <zephyr/bluetooth/hci.h>
+#include <zephyr/bluetooth/hci_vs.h>
 #include <zephyr/bluetooth/uuid.h>
+#include <zephyr/sys/byteorder.h>
 #include <zephyr/devicetree.h>
 #include <zephyr/drivers/hwinfo.h>
 #include <zephyr/drivers/sensor.h>
@@ -28,9 +30,12 @@
 #define OD_APP_VERSION         0x0100u
 #endif
 
-/* BLE adv interval units are 0.625 ms (same as nRF52840 Firmware). */
-#define OD_ADV_INTERVAL_MIN       BT_GAP_ADV_SLOW_INT_MIN /* 1600 = 1000 ms (~1 adv/s) */
-#define OD_ADV_INTERVAL_MAX       BT_GAP_ADV_SLOW_INT_MIN
+/* BLE adv interval units are 0.625 ms (same as nRF52840 Firmware). Matches the
+ * reference nRF window (NRF_ADV_INTERVAL_MIN/MAX, ble_init.cpp:48-49): a 160 ms
+ * floor for faster discovery up to a 1000 ms ceiling; the controller picks a
+ * value within the window. */
+#define OD_ADV_INTERVAL_MIN       256u                   /* 160 ms */
+#define OD_ADV_INTERVAL_MAX       BT_GAP_ADV_SLOW_INT_MIN /* 1600 = 1000 ms (~1 adv/s) */
 #define OD_ADV_BOOST_INTERVAL_MIN 32u   /* 20 ms */
 #define OD_ADV_BOOST_INTERVAL_MAX 48u   /* 30 ms */
 #define OD_ADV_BOOST_MS           3000u
@@ -46,6 +51,7 @@ static uint32_t s_adv_boost_until_ms;
 static uint8_t s_msd_loop_counter;
 static uint32_t s_last_adv_retry_ms;
 static uint8_t s_reboot_flag = 1; /* set after boot, cleared on first BLE connect */
+static uint8_t s_connection_requested; /* MSD status bit2; see opendisplay_ble_set_connection_requested */
 static uint8_t s_last_published_msd[MSD_PAYLOAD_LEN];
 static bool s_msd_published;
 static bool s_adv_was_boosted;
@@ -58,6 +64,7 @@ static bool s_adv_work_msd_publish;
 
 static int start_advertising(void);
 static bool publish_msd_to_advertising(void);
+static void apply_tx_power(uint8_t handle_type, uint16_t handle);
 
 static void adv_work_handler(struct k_work *work)
 {
@@ -142,10 +149,12 @@ static void update_msd_payload(void)
 	}
 	temperature_byte = (uint8_t)temp_encoded;
 	battery_voltage_low_byte = (uint8_t)(battery_voltage_10mv & 0xFFu);
-	/* Matches nRF52840 Firmware: bit1 rebootFlag, bit2 connectionRequested
-	 * (reserved, 0), bits 4-7 loop counter. */
+	/* Matches nRF52840 Firmware status byte (display_service.cpp:1293-1297):
+	 * bit0 battery high bit, bit1 rebootFlag, bit2 connectionRequested,
+	 * bits 4-7 loop counter. */
 	status_byte = (uint8_t)(((battery_voltage_10mv >> 8) & 0x01u) |
 				((s_reboot_flag & 0x01u) << 1) |
+				((s_connection_requested & 0x01u) << 2) |
 				((s_msd_loop_counter & 0x0Fu) << 4));
 
 	memset(msd_payload, 0, sizeof(msd_payload));
@@ -250,6 +259,15 @@ static void connected(struct bt_conn *conn, uint8_t err)
 	s_conn = bt_conn_ref(conn);
 	s_adv_active = false;
 	s_reboot_flag = 0;
+#if defined(CONFIG_BT_HCI_VS)
+	{
+		uint16_t conn_handle = 0;
+
+		if (bt_hci_get_conn_handle(conn, &conn_handle) == 0) {
+			apply_tx_power(BT_HCI_VS_LL_HANDLE_TYPE_CONN, conn_handle);
+		}
+	}
+#endif
 }
 
 static void disconnected(struct bt_conn *conn, uint8_t reason)
@@ -323,13 +341,65 @@ void opendisplay_ble_pipe_on_connection_closed(void)
 	opendisplay_pipe_on_connection_closed();
 }
 
+void opendisplay_ble_set_connection_requested(bool requested)
+{
+	s_connection_requested = requested ? 1u : 0u;
+}
+
 void opendisplay_ble_set_dynamic_byte(uint8_t index, uint8_t value)
 {
 	if (index < sizeof(dynamic_return)) {
 		dynamic_return[index] = value;
 	}
 }
 
+/*
+ * Apply the configured TX power (power_option.tx_power, dBm as a signed int8).
+ * The reference nRF52840 build calls Bluefruit.setTxPower(power_option.tx_power)
+ * once at init (ble_init.cpp:90). On Zephyr with the SoftDevice Controller there
+ * is no stable bt_le_* runtime API for this, so use the standard HCI vendor-
+ * specific Write_Tx_Power_Level command (as in Zephyr's hci_pwr_ctrl sample):
+ * the controller clamps the requested value to its supported set and returns the
+ * value it actually selected, which we log. handle_type selects advertising vs a
+ * specific connection.
+ */
+static void apply_tx_power(uint8_t handle_type, uint16_t handle)
+{
+#if defined(CONFIG_BT_HCI_VS)
+	int8_t requested = (int8_t)s_od_global_config.power_option.tx_power;
+	struct bt_hci_cp_vs_write_tx_power_level *cp;
+	struct bt_hci_rp_vs_write_tx_power_level *rp;
+	struct net_buf *buf;
+	struct net_buf *rsp = NULL;
+	int err;
+
+	buf = bt_hci_cmd_alloc(K_FOREVER);
+	if (buf == NULL) {
+		printf("[OD] tx_power: no HCI cmd buffer\r\n");
+		return;
+	}
+	cp = net_buf_add(buf, sizeof(*cp));
+	cp->handle = sys_cpu_to_le16(handle);
+	cp->handle_type = handle_type;
+	cp->tx_power_level = requested;
+
+	err = bt_hci_cmd_send_sync(BT_HCI_OP_VS_WRITE_TX_POWER_LEVEL, buf, &rsp);
+	if (err != 0) {
+		printf("[OD] tx_power set failed (type=%u req=%d dBm): %d\r\n",
+		       (unsigned)handle_type, (int)requested, err);
+		return;
+	}
+	rp = (struct bt_hci_rp_vs_write_tx_power_level *)rsp->data;
+	printf("[OD] tx_power type=%u requested=%d selected=%d dBm\r\n",
+	       (unsigned)handle_type, (int)requested, (int)rp->selected_tx_power);
+	net_buf_unref(rsp);
+#else
+	ARG_UNUSED(handle_type);
+	ARG_UNUSED(handle);
+	printf("[OD] tx_power: CONFIG_BT_HCI_VS disabled; not applied\r\n");
+#endif
+}
+
 static void apply_adv_interval(void)
 {
 	uint32_t now = k_uptime_get_32();
@@ -405,6 +475,8 @@ void opendisplay_ble_reload_config_from_nvm(void)
 		memset(&s_od_global_config, 0, sizeof(s_od_global_config));
 	}
 	flash_powerdown_from_config();
+	/* Re-apply advertising TX power in case the new config changed it. */
+	apply_tx_power(BT_HCI_VS_LL_HANDLE_TYPE_ADV, 0);
 }
 
 void opendisplay_ble_restart_advertising(void)
@@ -468,6 +540,7 @@ void opendisplay_ble_init(void)
 
 	opendisplay_button_init();
 	k_work_init_delayable(&s_adv_restart_work, adv_work_handler);
+	apply_tx_power(BT_HCI_VS_LL_HANDLE_TYPE_ADV, 0);
 	update_msd_payload();
 	(void)start_advertising();
 	printf("[OD] BLE ready as %s\r\n", s_dev_name);

src/opendisplay_ble.h

@@ -25,6 +25,13 @@ void opendisplay_ble_copy_msd_bytes(uint8_t out[16]);
 void opendisplay_ble_update_msd(bool refresh_advertising);
 float opendisplay_ble_get_chip_temperature(void);
 
+/* Sets MSD status bit2 (connectionRequested). Mirrors the reference nRF52840
+ * flag (main.h:127, display_service.cpp:1296): a device-side request for the
+ * host to connect. No producer exists yet on this branch (nor in the reference,
+ * where it is reserved-for-future and stays 0); this is the hook a future
+ * feature / sibling branch wires. */
+void opendisplay_ble_set_connection_requested(bool requested);
+
 bool opendisplay_ble_pipe_notify(const uint8_t *data, uint16_t len);
 bool opendisplay_ble_pipe_notify_enabled(void);
 void opendisplay_ble_pipe_on_write(const uint8_t *data, uint16_t len, bool write_cmd);

src/opendisplay_button.c

@@ -22,6 +22,16 @@ typedef struct {
 static ButtonState s_buttons[MAX_BUTTONS];
 static uint8_t s_button_count;
 
+/* Set from GPIO ISR context (both-edges interrupt). The ISR does NO I2C/BLE
+ * work: it only raises this flag, which opendisplay_button_process() consumes on
+ * the main loop. Polling remains as a fallback in case an edge is missed. */
+static volatile bool s_button_irq_pending;
+
+static void button_irq_handler(void)
+{
+  s_button_irq_pending = true;
+}
+
 static bool read_logical_pressed(const ButtonState *btn)
 {
   bool level = nrf54_gpio_read(btn->pin) != 0;
@@ -66,17 +76,30 @@ void opendisplay_button_init(void)
       btn->byte_index = input->button_data_byte_index;
       btn->pin = pin;
       btn->inverted = (input->invert & (1u << pin_idx)) != 0u;
-      nrf54_gpio_configure_input(pin, (input->pullups & (1u << pin_idx)) != 0u, false);
+      bool pull_up = (input->pullups & (1u << pin_idx)) != 0u;
+      bool pull_down = (input->pulldowns & (1u << pin_idx)) != 0u;
+      nrf54_gpio_configure_input(pin, pull_up, pull_down);
       btn->current_state = read_logical_pressed(btn) ? 1u : 0u;
       btn->initialized = true;
-      printf("[OD] button id=%u pin=0x%02X byte=%u\r\n", (unsigned)btn->button_id, (unsigned)pin,
-             (unsigned)btn->byte_index);
+      /* Attach a both-edges interrupt (reference uses CHANGE, device_control.cpp:604).
+       * On failure we still have the polling path in _process(). */
+      if (nrf54_gpio_configure_interrupt(pin, button_irq_handler) != 0) {
+        printf("[OD] button pin=0x%02X interrupt setup failed; polling only\r\n",
+               (unsigned)pin);
+      }
+      printf("[OD] button id=%u pin=0x%02X byte=%u pull=%s\r\n", (unsigned)btn->button_id,
+             (unsigned)pin, (unsigned)btn->byte_index,
+             pull_up ? "up" : (pull_down ? "down" : "none"));
     }
   }
 }
 
 void opendisplay_button_process(void)
 {
+  /* Consume any interrupt signal. The actual state change is detected by the
+   * poll below (edge-agnostic), which also covers a missed/coalesced edge. */
+  s_button_irq_pending = false;
+
   for (uint8_t i = 0; i < s_button_count; i++) {
     ButtonState *btn = &s_buttons[i];
     bool pressed;