Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,23 @@ In `key_action.h`:
## Code Review Philosophy

Focus on functional aspects, not nitpicks. Only flag magic constants if they're used in multiple places and may need future changes. Don't require comments unless truly necessary.

## UHK80 / Zephyr Dev Environment — Findings

Key gotchas discovered while setting up:

- **Re-run `./build.sh update` after any branch/manifest switch.** The west modules (zephyr, nrf, …)
are pinned per `west_nrfsdk.yml`; if they don't match the current checkout, CMake configure fails
with `Error finding board: uhk-80-right ... Malformed "build" section ... SchemaError`. This is a
Zephyr version mismatch, not a `board.yml` bug — `./build.sh update` (west update + patch) fixes it.
- **Single-device builds run in the foreground** (`./build.sh right build`), which is easiest for
testing; multi-device (`left right dongle`) fans out into tmux `buildsession` panes.
- **Dongle link depends on `keyboard/` sources.** `right/src/slave_drivers/kboot_driver.c` is compiled
for all UHK80 targets, but `device/src/CMakeLists.txt` only compiles `keyboard/*.c` (which includes
`uart_modules.c`) for left/right, **not** the dongle. So any symbol `kboot_driver.c` pulls from
`uart_modules.c` (e.g. the `KbootUart_*` UART-transport functions) must be stubbed/guarded for the
dongle or the dongle link fails with `undefined reference`.
- **Agent npm shadow.** `lib/agent` is nested inside this repo; the firmware root's `node_modules/npm`
(11.8.0, pinned by the root lockfile) shadows nvm's npm during the Agent build and trips its
`check-node-version` (>=11.13.0). Fix: `npm install npm@11.13.0 --no-save --no-package-lock` in the
firmware root. Details in the reference doc above.
44 changes: 29 additions & 15 deletions device/prj.conf.overlays/nrf_shared.conf
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ CONFIG_INIT_STACKS=y
CONFIG_LOG=y
CONFIG_LOG_CMDS=y
CONFIG_LOG_MODE_DEFERRED=y
CONFIG_LOG_BUFFER_SIZE=2048
CONFIG_LOG_BACKEND_SHOW_COLOR=y
CONFIG_LOG_RUNTIME_FILTERING=y

Expand Down Expand Up @@ -38,19 +37,31 @@ CONFIG_BT_FILTER_ACCEPT_LIST=y
CONFIG_BT_CTLR_SDC_MAX_CONN_EVENT_LEN_DEFAULT=2500
CONFIG_BT_CTLR_SDC_CENTRAL_ACL_EVENT_SPACING_DEFAULT=2500

# increase these to make multiple connections more reliable
# this is a generic ai advice.
CONFIG_BT_ATT_TX_COUNT=10
CONFIG_BT_CONN_TX_MAX=6
CONFIG_BT_L2CAP_TX_BUF_COUNT=12

# negotiate larger MTU for NUS
# BLE buffer counts, sized for ~2 active links (L=2) with double buffering
# (D=2), no HCI fragmentation (LL unit == L2CAP MTU, so frags/PDU F=1). Each
# line notes the constraint it must satisfy. BT_BUF_ACL_TX_COUNT (the HCI TX
# credit window) is left at its default 3: it is capped by the controller's
# BT_CTLR_SDC_TX_PACKET_COUNT and 3 credits already cover 2 links.
# ATT TX PDUs in flight: >= L*D (freed on HCI handoff, not air-ack)
CONFIG_BT_ATT_TX_COUNT=5
# L2CAP signalling/SMP only (no CoC): a handful
CONFIG_BT_L2CAP_TX_BUF_COUNT=4
# HCI event RX: > BT_BUF_ACL_TX_COUNT (build assert)
CONFIG_BT_BUF_EVT_RX_COUNT=6
# ACL RX reassembly: 1+EXTRA >= L+1 (one/link + driver spare)
CONFIG_BT_BUF_ACL_RX_COUNT_EXTRA=2

# NUS packet sizing. We cap the link packet at MAX_LINK_PACKET_LENGTH=128
# (link_protocol.h) rather than the DLE maximum, to save RAM and cut BLE latency.
# ACL buffer = MAX_LINK_PACKET_LENGTH + L2CAP(4) + ATT(3) = 135
# L2CAP MTU = MAX_LINK_PACKET_LENGTH + ATT(3) = 131
# bt_conn.c caps the negotiated DLE tx length to CONFIG_BT_BUF_ACL_TX_SIZE.
CONFIG_BT_USER_DATA_LEN_UPDATE=y
CONFIG_BT_BUF_ACL_RX_SIZE=251
CONFIG_BT_BUF_ACL_TX_SIZE=251
CONFIG_BT_L2CAP_TX_MTU=247
CONFIG_BT_BUF_ACL_RX_SIZE=135
CONFIG_BT_BUF_ACL_TX_SIZE=135
CONFIG_BT_L2CAP_TX_MTU=131

# allow BLE buffering
# inbound long-write (config transfer) queue depth
CONFIG_BT_ATT_PREPARE_COUNT=4

# store BLE link keys in flash
Expand All @@ -61,9 +72,12 @@ CONFIG_FLASH_MAP=y
CONFIG_SETTINGS=y
CONFIG_NVS=y

# prevent dropped messages
CONFIG_LOG_BUFFER_SIZE=4096
CONFIG_SEGGER_RTT_BUFFER_SIZE_UP=4096
# Log/RTT buffers kept small to save RAM (we are tight on RAM). If these small
# buffers drop messages under bursty logging, raise the logging thread priority
# at runtime with the "uhk log priority high" shell command rather than enlarging
# the buffers.
CONFIG_LOG_BUFFER_SIZE=1024
CONFIG_SEGGER_RTT_BUFFER_SIZE_UP=512

CONFIG_SERIAL=y

Expand Down
7 changes: 5 additions & 2 deletions device/src/bt_conn.c
Original file line number Diff line number Diff line change
Expand Up @@ -291,10 +291,13 @@ static void enableDataLengthExtension(struct bt_conn *conn) {
data_len = BT_LE_DATA_LEN_PARAM_MAX;

/**
* This configures actual transmission length.
* Configures actual transmission length. Cap the payload to our reduced ACL
* buffer (CONFIG_BT_BUF_ACL_TX_SIZE) rather than the DLE max of 251: we never
* send larger packets and shorter PDUs mean lower latency.
*
* We don't want it too high in order to prevent scheduling conflicts between multiple links.
* tx_max_time kept low to avoid scheduling conflicts between multiple links.
* */
data_len->tx_max_len = CONFIG_BT_BUF_ACL_TX_SIZE;
data_len->tx_max_time = 2500;

int err = bt_conn_le_data_len_update(conn, data_len);
Expand Down
10 changes: 7 additions & 3 deletions device/src/link_protocol.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,16 @@

// Macros:

// 251 = maximum BLE packet length with data length extension
// Maximum deserialized message length (header + payload) that fits in a single
// BLE notification / UART bridge frame. We deliberately keep this well below the
// DLE maximum (244 with a 251-byte ACL buffer) to save RAM (messenger-queue
// regions, nus/uart buffers) and to shorten BLE packets for lower latency.
// 4 bytes reserved for L2CAP header
// 3 bytes reserved for ATT header
// https://devzone.nordicsemi.com/f/nordic-q-a/111900/maximum-nus-packet-payload-with-ble-data-length-extensio
// Should equal `CONFIG_BT_BUF_ACL_RX_SIZE - L2CAP_HEADER_SIZE - ATT_HEADER_SIZE`, otherwise something is wrong
#define MAX_LINK_PACKET_LENGTH 244
// Should equal `CONFIG_BT_BUF_ACL_RX_SIZE - L2CAP_HEADER_SIZE - ATT_HEADER_SIZE`
// (i.e. 135 - 4 - 3), otherwise something is wrong.
#define MAX_LINK_PACKET_LENGTH 128

// Typedefs:

Expand Down
Loading
Loading