Skip to content

Serialize BLE access, stop replaced command loops, release the adapter when idle - #174

Merged
wimaha merged 5 commits into
mainfrom
fix/ble-serialization
Oct 18, 2026
Merged

wimaha merged 5 commits into
mainfrom
fix/ble-serialization

Conversation

@wimaha

@wimaha wimaha commented Oct 18, 2026 •

Copy link
Copy Markdown
Owner

Why

#161 by @AloisKlingler lets the proxy share the Bluetooth adapter with other BlueZ services (#158) by calling ble.CloseAdapter() whenever the command loop is idle. That is only safe if no BLE connection is open at that moment, and today that isn't guaranteed:

  • body_controller_state and SendKeysToVehicle open their own connections next to the command loop. CloseAdapter stops the device without closing existing connections, so a parallel request would lose its adapter.
  • The active BleControl was a plain global. A key change set it to nil and replaced it while handlers read it twice (checkBleControl, then PushCommand), which could cause a nil dereference. Every SetupBleControl() also started a new loop without stopping the old one.

This PR fixes those first and then adds #161 on top. It replaces #161, which no longer applies after the refactoring in #169. Credit goes to @AloisKlingler as co-author.

What

One connection at a time

  • body_controller_state goes through the command queue (VCSEC only, so it still never wakes the car). If a connection is already open, it is answered on it right away without a new scan. The 15 s limit stays.
  • A command that needs Infotainment arriving on a VCSEC-only connection reconnects with the usual wake-up and session logic.
  • SendKeysToVehicle and the loop share bleMu, held from dial to disconnect.

Key changes

  • The instance is an atomic.Pointer. Handlers read it once per request (control.Instance()).
  • SetupBleControl stops the previous loop. Stop() closes its connection and answers queued and later commands with an error instead of leaving them hanging.

From #161

  • connectionTimeout: how long a connection stays open after a command. The default stays at 29 s, so behaviour is unchanged.
  • releaseAdapterWhenIdle (off by default): calls ble.CloseAdapter() when the loop is idle, under bleMu. SendKeysToVehicle also releases the adapter afterwards.

Errors no longer reported as success

  • Non-retried errors were returned as result: true, for example set_charging_amps without a body. They are now reported.
  • Missing or invalid bodies for set_charging_amps, set_charge_limit and set_sentry_mode are InvalidInputErrors and are checked before queuing, so they never contact the car.

Docs: env var docs, a README troubleshooting section on sharing the adapter, and CHANGELOG entries. All new sections are marked *(unreleased)*.

Behaviour changes to note

  • After body_controller_state, the connection now stays open for connectionTimeout like after any command, instead of closing at once. A following command reuses it.
  • A failed non-retried send closes the connection, like the rear-trunk case already did. Invalid input keeps it open, as before.

BLE library for #158

wimaha/ble_BleConnectFix#1 (HCIDEVUP in Close()) is merged, and go.mod now uses it (b933489). The module diff is only that change in linux/hci/socket/socket.go. With releaseAdapterWhenIdle=true, BlueZ sees the adapter again after each release.

Review follow-up

  • Push never blocks. A full queue (50 requests) is answered with an error right away, so Stop() (and a key change) can no longer be held up by a waiting push.
  • body_controller_state in the queue: documented in the README API section and under CHANGELOG "Changed". A caller that gave up after 15 s is never sent to the car: the loop checks the request context before dialing or sending. A test covers this.
  • SendKeysToVehicle asks the loop to close its idle connection (RequestClose) instead of waiting up to connectionTimeout, and gives up after 30 s with a clear message.
  • CHANGELOG "Changed" now covers the queue behaviour, the stricter error reporting, the full-queue answer and the BLE library update.

Second round

  • Async commands (no wait=true) that cannot be queued (queue full, key change) now return 503 with the reason instead of "will be processed shortly". Commands dropped later during a key change are logged as not executed.
  • Key change while the car is out of range: Stop() now also cancels a running connection attempt (scan, retry waits), so the old loop frees Bluetooth at once and SendKeysToVehicle does not run into "Bluetooth is busy". A test covers this; without the fix it fails.
  • CHANGELOG "Breaking": the stricter result: false, the async rejection and the 15 s body_controller_state limit are listed as possibly breaking for integrations.
  • arm/v6 builds as well (GOARM=6). ble.CloseAdapter() is a no-op when no adapter is open (if device != nil in vehicle-command), so calling it at loop start is harmless.

Testing

  • go vet ./... and go test -race ./.... The new tests ran 10× under -race without failure.
  • New tests in control_lifecycle_test.go:
    • Stop answers queued and later commands, and closes an open connection.
    • A replaced loop never has two connections open at once (counted).
    • Swapping the instance is race-free.
    • body-controller-state reuses the open connection, and the following Infotainment command reconnects and wakes.
    • The adapter is released only while idle, and never by default.
    • Non-retried errors are reported.
    • A full queue answers at once and Stop does not hang.
    • A canceled request never reaches the car.
    • RequestClose ends an idle connection.
    • lockBLE gives up after its timeout.
  • HTTP tests: invalid bodies for the three commands return 503 without contacting the car.
  • Mutation check: removing bleMu, the Stop case or the Infotainment check makes the matching test fail.
  • Linux builds (arm64, armv7) with the bumped BLE library.
  • Local binary without a car: env vars are logged. After two key generations and two activations, exactly one loop is running and three were stopped.
  • Not tested: a real Raspberry Pi with a car and evcc, with and without releaseAdapterWhenIdle. @AloisKlingler, could you check this branch in your shared-adapter setup?

🤖 Generated with Claude Code

wimaha and others added 2 commits October 18, 2026 23:03
…r when idle

Only one BLE connection may be open at a time, and the adapter may only be
released when none is. Neither was guaranteed:

- body_controller_state and SendKeysToVehicle opened their own connections
  next to the command loop. body_controller_state now goes through the
  queue (VCSEC only, so it still never wakes the car) and is answered right
  away on an open connection. SendKeysToVehicle and the loop share bleMu,
  held from dial to disconnect. A command that needs Infotainment on a
  VCSEC-only connection reconnects with the usual wake-up logic.
- The active BleControl was a plain global that a key change set to nil and
  replaced while handlers read it twice. It is now an atomic pointer read
  once per request. SetupBleControl stops the previous loop instead of
  leaving it running; Stop closes its connection and answers queued and
  later commands with an error instead of leaving them hanging.

From #161: connectionTimeout makes the connection hold configurable
(default 29 s as before), and releaseAdapterWhenIdle (off by default) calls
ble.CloseAdapter whenever the loop is idle, under bleMu, so other BlueZ
services on the host can use the adapter (#158).

Errors that are not retried are now reported instead of returned as
success. Missing or invalid bodies for set_charging_amps, set_charge_limit
and set_sentry_mode are InvalidInputErrors, checked before queuing, so they
answer with an error and never contact the car.

Co-authored-by: AloisKlingler <11095760+AloisKlingler@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Environment variable docs, a README troubleshooting section on sharing the
Bluetooth adapter, and CHANGELOG entries under Unreleased.

Co-authored-by: AloisKlingler <11095760+AloisKlingler@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

🟡 Merge after the to-dos

Good fix for the double-connection and nil-pointer problems, and the new options are off or unchanged by default. Two behaviour changes can still be noticed by evcc and Home Assistant, so please check them on a real car first.

Impact on existing users: body_controller_state now waits in the same queue as other commands, so it can be slow or time out (15 s) while another command is running or retrying. Some errors that used to come back as result: true now come back as failures.

To-do for the author

  • Decide what body_controller_state should do when the queue is busy (e.g. a wake_up or a retry loop with the car out of range). Before, it answered on its own connection. Now evcc may get an error instead of a state. Document it or add a fast path.
  • Check that Stop() cannot hang when the command queue (50) is full (see details).
  • Give SendKeysToVehicle a clear wait limit or message: the dashboard can now block up to connectionTimeout (29 s) before the 15 s timeout even starts.
  • Add a line in the CHANGELOG "Changed" section about the body_controller_state queue behaviour and the stricter error reporting.

Test before merging: On a Pi with a car: run evcc polling plus a wake_up/charge_start, then change the key in the dashboard. With and without releaseAdapterWhenIdle=true.

Technical details
  • internal/api/handlers/tesla.go (BodyControllerState): it used to dial directly. Now it goes through bc.Push, so it waits behind any running command or retry. After 15 s the handler returns an error, but the command stays queued and still runs later (the car is contacted even though the caller gave up; it only reads VCSEC, so it does not wake the car).
  • control.go Push: holds stopMu.RLock while doing a blocking send into commandStack (cap 50). If the queue is full, Stop() waits for the write lock. Go's RWMutex then also blocks new readers. SetupBleControl holds setupMu while calling Stop(), so a key change in the dashboard could hang. I did not reproduce this; please check or use a select with bc.done.
  • control.go ExecuteCommand: every non-retried error is now returned. Callers who relied on e.g. set_charging_amps without a valid body returning true now see false. This is correct, but it is a visible API change.
  • key.go SendKeysToVehicle: bleMu.Lock() has no timeout. It waits for the open connection of the loop.
  • Not verified: real BLE behaviour of ble.CloseAdapter() and re-init each idle loop (needs the go-ble fix from the linked PR for BlueZ to benefit). No go.mod change here.
  • Good: defaults keep 29 s timeout; the new env vars are additive; API routes and JSON shape are unchanged.

…ection

Review follow-up for #174:

- Push held the read lock while sending into a full queue, so Stop (and with
  it a key change) waited until the loop drained it, and RWMutex then blocked
  every further push too. Push no longer blocks: a full queue (50 requests,
  the car has been unreachable for a while) is answered with an error.
- SendKeysToVehicle asks the loop to close its connection (RequestClose)
  instead of waiting up to connectionTimeout for it, and gives up after 30 s
  with a clear message.
- Test that a caller who gave up (body_controller_state after 15 s) is never
  sent to the car: the loop checks the request context before dialing.
- Document the queue behaviour of body_controller_state and the stricter
  error reporting under "Changed".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
wimaha/ble_BleConnectFix#1 (by @AloisKlingler) issues HCIDEVUP when the HCI
user channel socket is closed. Without it the adapter stays down after
ble.CloseAdapter and BlueZ never sees it again, so releaseAdapterWhenIdle
had no effect for other services (#158). The module diff is only that
change in linux/hci/socket/socket.go.

Co-authored-by: AloisKlingler <11095760+AloisKlingler@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@wimaha wimaha self-assigned this Oct 18, 2026
@wimaha wimaha added the claude-review Add to request a new Claude review (removed automatically) label Oct 18, 2026
@github-actions

Copy link
Copy Markdown

🟡 Merge after the to-dos

Good fix: one BLE connection at a time, and old loops are stopped after a key change. Defaults stay the same (29 s, adapter kept). One real problem: commands sent without wait=true can be lost and still look like success.

Impact on existing users: Some errors that used to return result: true now return result: false (HTTP 503). Also, body_controller_state can now fail after 15 s if the proxy is busy retrying a command. evcc / Home Assistant may log new errors in these cases.

To-do for the author

  • Async commands (no wait=true): when the queue is full or the loop was stopped, the command is dropped, but the API still says "will be processed shortly". Return an error to the caller, or at least log it clearly.
  • Add the stricter result: false behaviour and the new 15 s body_controller_state limit to the release notes as a possible breaking change for integrations (CHANGELOG already has it; consider making it more visible).
  • Check that a key change while the car is out of range does not leave SendKeysToVehicle failing with "Bluetooth is busy" for 30 s.

Test before merging: On a real Pi with evcc: poll body_controller_state while a command retries (car out of range), change the active key in the dashboard, and run once with releaseAdapterWhenIdle=true. Not tested with a real car yet, as the PR says.

Technical details
  • internal/api/handlers/tesla.go ~L124 (PushCommand(..., nil, ...) then Result = true) and control.go Push/failCommand: with Response == nil, failCommand returns silently. A full queue (50) or a stopped loop drops the command with no signal. Before, a full queue blocked the handler. Same for retryCommand in the Loop defer and failQueued after a key change.
  • control.go ExecuteCommand: every non-retried error (unknown command, key-file errors in add-key paths, request body is nil) is now returned instead of success. Also handleCommand now closes the connection on these, so the next command reconnects. Intended, but it is a visible change for callers.
  • body_controller_state now waits behind the queue. Before it opened its own connection, so it worked while another command was retrying. Now it may time out at 15 s. Documented in the PR.
  • lockBLE / bleMu: the loop holds bleMu for the whole dial, including the scan. RequestClose does not interrupt a connect attempt, so SendKeysToVehicle can wait up to 30 s and then fail while the car is unreachable. Could not verify the scan duration.
  • go.mod: ble_BleConnectFix bumped to v0.0.0-20261018210806-b933489a83fc. I did not inspect the module; the PR says only HCIDEVUP in Close() changed. Please confirm arm/v6 and arm/v7 build (PR tested arm64 and armv7 only).
  • Not verified: behaviour of ble.CloseAdapter() when no adapter is open (called at loop start with releaseAdapterWhenIdle=true).
  • Fine: new env vars are additive, invalid values fall back to defaults; no route, key-path or Docker changes.

@github-actions github-actions Bot removed the claude-review Add to request a new Claude review (removed automatically) label Oct 18, 2026
Second review follow-up for #174:

- A command sent without wait=true that could not be queued (queue full,
  loop stopped by a key change) was dropped while the API answered "will be
  processed shortly". Push now returns the error and the handler reports it
  as result=false. Commands dropped later (queued during a key change) are
  logged as not executed.
- Stop ended an open connection but not a running connection attempt, so
  after a key change with the car out of range the old loop kept Bluetooth
  busy through its scans and retries, and SendKeysToVehicle could time out
  with "Bluetooth is busy". Stop now cancels the attempt as well.
- CHANGELOG lists the stricter error reporting, the async rejection and the
  body_controller_state queue limit under "Breaking" as possibly breaking
  for integrations.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@wimaha

wimaha commented Oct 18, 2026

Copy link
Copy Markdown
Owner Author

Should fix: #158

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant