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
19 changes: 11 additions & 8 deletions FINDINGS.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,14 +92,17 @@ machinery exists.

### P5. 🟠 BLE throughput: stop-and-wait, write-with-response, fixed chunk sizes

- Every chunk incurs a GATT write-with-response (`transport/connection.py:262-266`,
`response=True`) **and** a firmware ACK notification before the next chunk
(`device.py:1563-1598`). Over an ESPHome/HA proxy that is ~2 RTTs per 154/230-byte
chunk — hundreds of serialized round-trips for a 50 KB image. This is the dominant
transfer-time limiter. `PIPELINE_CHUNKS` exists (`protocol/commands.py:51`) but is
hardcoded to 1 and unused. If firmware tolerates it, a small in-flight window or
write-without-response for 0x71 data with periodic ACK checkpoints would multiply
throughput. (Firmware ACKs every 0x71, so this needs a firmware-coordinated change.)
- **Partially addressed:** 0x71 data chunks now use BLE Write Without Response
(`transport/connection.py` `write_command(..., response=False)`, opted in from
`_send_data_chunks`/`_send_partial_chunks`), removing the GATT write-with-response
round-trip. The firmware ACK notification per chunk is still awaited (stop-and-wait),
which preserves flow control and needs no firmware change — so exactly one write is in
flight at a time. This roughly halves the RTTs per chunk. The characteristic is probed
for the `write-without-response` property and falls back to write-with-response if absent.
- Remaining: the per-chunk firmware ACK is still serialized. A true in-flight window /
periodic ACK checkpoints would multiply throughput further, but that DOES need a
firmware-coordinated change (firmware ACKs every 0x71, and the ESP32 command queue is
only 5 deep). `PIPELINE_CHUNKS` (`protocol/commands.py:52`) stays 1 for now.
- No MTU negotiation: `CHUNK_SIZE = 230` / `ENCRYPTED_CHUNK_SIZE = 154`
(`protocol/commands.py:47-48`) are constants; `client.mtu_size` is never consulted, so
links that negotiate 247+ leave throughput on the table.
Expand Down
32 changes: 32 additions & 0 deletions src/opendisplay/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -703,6 +703,37 @@ async def _reboot(device_kwargs: dict[str, Any]) -> None:
_console.print("Reboot command sent. Device will restart.")


# ── sleep ─────────────────────────────────────────────────────────────────────


def _add_sleep_parser(subparsers: argparse._SubParsersAction[argparse.ArgumentParser]) -> None:
p = subparsers.add_parser("sleep", help="Put the device into deep sleep (command 0x0052)")
_add_device_options(p)
p.set_defaults(func=_cmd_sleep)


def _cmd_sleep(args: argparse.Namespace) -> None:
key = _parse_hex_key(args.key)
_run(_sleep(_device_kwargs(args.device, key, args.timeout)))


async def _sleep(device_kwargs: dict[str, Any]) -> None:
slept = False
with _spinner() as progress:
progress.add_task("Connecting...", total=None)
try:
async with OpenDisplayDevice(**device_kwargs) as device:
await device.deep_sleep()
slept = True
except (BLEConnectionError, BLETimeoutError):
if not slept:
_error("BLE connection failed before deep sleep command could be sent.")
# else: expected drop after the device sleeps
except OpenDisplayError as exc:
_handle_ble_error(exc)
_console.print("Deep sleep command sent. Device will sleep until its next wake.")


# ── export-config ─────────────────────────────────────────────────────────────


Expand Down Expand Up @@ -792,6 +823,7 @@ def main() -> None:
_add_info_parser(subparsers)
_add_upload_parser(subparsers)
_add_reboot_parser(subparsers)
_add_sleep_parser(subparsers)
_add_export_config_parser(subparsers)
_add_write_config_parser(subparsers)

Expand Down
85 changes: 79 additions & 6 deletions src/opendisplay/device.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
build_authenticate_step1,
build_authenticate_step2,
build_buzzer_activate_command,
build_deep_sleep_command,
build_direct_write_data_command,
build_direct_write_end_command,
build_direct_write_end_with_etag,
Expand Down Expand Up @@ -558,17 +559,24 @@ def _on_ble_disconnect(self) -> None:
_LOGGER.debug("Link to %s dropped; clearing session state", self.mac_address)
self._clear_session()

async def _write(self, data: bytes) -> None:
"""Write a command, encrypting it if an active session exists."""
async def _write(self, data: bytes, response: bool = True) -> None:
"""Write a command, encrypting it if an active session exists.

Args:
data: Plaintext command frame (opcode + payload).
response: Passed through to the transport. False requests a BLE Write
Without Response (used for 0x71 data chunks); applies whether or not
the frame is encrypted.
"""
if self._session_key is not None and self._session_id is not None:
await self._reauthenticate_if_needed()
cmd = data[:2]
payload = data[2:]
encrypted = encrypt_command(self._session_key, self._session_id, self._nonce_counter, cmd, payload)
self._nonce_counter += 1
await self._conn.write_command(encrypted)
await self._conn.write_command(encrypted, response=response)
else:
await self._conn.write_command(data)
await self._conn.write_command(data, response=response)

async def _reauthenticate_if_needed(self) -> None:
"""Re-authenticate proactively at 90% of session_timeout_seconds."""
Expand Down Expand Up @@ -950,6 +958,67 @@ async def reboot(self) -> None:
# Device will reset immediately - no ACK expected
_LOGGER.info("Reboot command sent to %s - device will reset (connection will drop)", self.mac_address)

@_serialized
async def deep_sleep(self) -> None:
"""Put the device into deep sleep (command 0x0052).

Supported on ESP32 and Silabs Flex; nRF targets do not implement deep
sleep. The command is sent encrypted when an active session exists.

The firmware's exact behavior depends on the target, and this method
tolerates all of them — in every supported case the BLE link drops during
or right after the command, so a disconnect (or a missing ACK) is treated
as success, mirroring reboot() and trigger_dfu_bootloader():

- ESP32 with a D-FF power latch: firmware ACKs 0x0052, then powers off
after ~100 ms (the link drops).
- ESP32 without a power latch: firmware enters deep sleep immediately,
tearing down BLE with no ACK (the write or read fails as the link drops).
- Silabs Flex: firmware ACKs 0x0052, then closes the connection and enters
EM4 (wake on button/NFC).

Raises:
ProtocolError: If the device explicitly reports that deep sleep is not
supported (protocol error frame 0xFF52).
"""
from .exceptions import BLEConnectionError

_LOGGER.debug("Sending deep sleep command (0x0052) to device %s", self.mac_address)

try:
await self._write(build_deep_sleep_command())
except BLEConnectionError as exc:
# An ESP32 without a power latch tears down BLE synchronously as it
# enters deep sleep, so the write-with-response confirmation can fail
# (e.g. a GATT/disconnect error over a Bluetooth proxy) even though the
# command was delivered. Treat that as the device having gone to sleep.
_LOGGER.debug(
"Deep sleep write did not confirm (expected — device sleeps before responding): %s",
exc,
)
_LOGGER.info("Deep sleep command sent to %s — device is sleeping (connection dropped)", self.mac_address)
return

# Targets that ACK before sleeping (ESP32 power-latch, Silabs Flex) reply
# with 0x0052 and then drop the link; a device that does not support the
# command replies with the 0xFF52 error frame (protocol: 0xFF [command_low]).
# A disconnect or timeout here means the device slept without acking.
try:
response = await self._read(self.TIMEOUT_ACK)
except (BLEConnectionError, BLETimeoutError) as exc:
_LOGGER.debug(
"No deep sleep ACK (expected — device dropped the link or sleeps silently): %s",
exc,
)
_LOGGER.info("Deep sleep command sent to %s — device is sleeping", self.mac_address)
return

if len(response) >= 2 and unpack_command_code(response) == 0xFF52:
raise ProtocolError("Device reported deep sleep is not supported (command 0x0052)")

validate_ack_response(response, CommandCode.DEEP_SLEEP)
_LOGGER.info("Deep sleep command acknowledged by %s — device is sleeping", self.mac_address)

@_serialized
async def trigger_dfu_bootloader(self) -> None:
"""Trigger the DFU bootloader on nRF devices (command 0x0051).
Expand Down Expand Up @@ -1532,7 +1601,8 @@ async def _send_partial_chunks(
offset = 0
while offset < len(remaining):
chunk = remaining[offset : offset + chunk_size]
await self._write(build_direct_write_data_command(chunk))
# Write Without Response; the per-chunk ACK read below keeps flow control.
await self._write(build_direct_write_data_command(chunk), response=False)
ack = await self._read(self.TIMEOUT_ACK)
nack = parse_nack(ack)
if nack is not None:
Expand Down Expand Up @@ -1817,7 +1887,10 @@ async def _send_data_chunks(
chunk_size = ENCRYPTED_CHUNK_SIZE if self._session_key is not None else CHUNK_SIZE
chunk_data = image_data[bytes_sent : bytes_sent + chunk_size]

await self._write(build_direct_write_data_command(chunk_data))
# Send the data chunk without waiting for the ATT write confirmation
# (Write Without Response). Flow control is preserved by the per-chunk
# application ACK read below, so only one write is ever in flight.
await self._write(build_direct_write_data_command(chunk_data), response=False)
bytes_sent += len(chunk_data)
chunks_sent += 1

Expand Down
9 changes: 9 additions & 0 deletions src/opendisplay/models/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,15 @@ def power_mode_enum(self) -> PowerMode | int:
except ValueError:
return self.power_mode

@property
def deep_sleep_enabled(self) -> bool:
"""Return True if the device is configured for deep sleep.

Mirrors the firmware's own sleep-entry condition: battery power mode
(PowerMode.BATTERY) with a non-zero timer-wake interval.
"""
return self.power_mode == PowerMode.BATTERY and self.deep_sleep_time_seconds > 0

@property
def capacity_estimator_enum(self) -> CapacityEstimator | int:
"""Get battery chemistry estimator as enum, or raw int if unknown."""
Expand Down
2 changes: 2 additions & 0 deletions src/opendisplay/protocol/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
build_authenticate_step1,
build_authenticate_step2,
build_buzzer_activate_command,
build_deep_sleep_command,
build_direct_write_data_command,
build_direct_write_end_command,
build_direct_write_end_with_etag,
Expand Down Expand Up @@ -49,6 +50,7 @@
"build_read_config_command",
"build_read_fw_version_command",
"build_enter_dfu_command",
"build_deep_sleep_command",
"build_reboot_command",
"build_write_config_command",
"build_direct_write_start_compressed",
Expand Down
27 changes: 26 additions & 1 deletion src/opendisplay/protocol/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class CommandCode(IntEnum):
DIRECT_WRITE_PARTIAL_START = 0x0076 # Start a partial update transfer (stream via 0x71)
BUZZER_ACTIVATE = 0x0077 # Host→device: trigger buzzer pattern (firmware 1.61+)
ENTER_DFU = 0x0051 # Trigger DFU bootloader mode (nRF only)
DEEP_SLEEP = 0x0052 # Enter deep sleep now (ESP32 timer-wake / Silabs EM4; nRF unsupported)


# Protocol constants
Expand All @@ -48,7 +49,9 @@ class CommandCode(IntEnum):
ENCRYPTED_CHUNK_SIZE = 154 # Maximum data bytes per chunk when session is active
# Encrypted packet: cmd(2)+nonce(16)+len(1)+data(154)+tag(12) = 185 bytes
CONFIG_CHUNK_SIZE = 200 # Maximum config chunk size (verified from firmware)
PIPELINE_CHUNKS = 1 # Wait for ACK after each chunk
PIPELINE_CHUNKS = 1 # One 0x71 write in flight at a time; ACK awaited after each chunk
# NOTE: 0x71 data chunks use BLE Write Without Response (no ATT confirmation), but the
# application-layer per-chunk ACK is still awaited, so only one write is ever outstanding.

# Upload protocol constants
MAX_COMPRESSED_SIZE = 50 * 1024 # Standard firmware buffer (nRF, ~50KB)
Expand Down Expand Up @@ -97,6 +100,28 @@ def build_enter_dfu_command() -> bytes:
return CommandCode.ENTER_DFU.to_bytes(2, byteorder="big")


def build_deep_sleep_command() -> bytes:
"""Build command to put the device into deep sleep (command 0x0052).

Supported on ESP32 (enters timer-wake deep sleep, or releases the D-FF power
latch when one is configured) and Silabs Flex (arms EM4 button/NFC wake and
sleeps once the BLE connection closes). nRF targets do not implement deep
sleep and only log the command.

The response behavior varies by target and is best-effort — callers should
tolerate the connection dropping during or right after the command:
- ESP32 with a power latch: replies 0x0052, then powers off after ~100 ms.
- ESP32 without a power latch: enters deep sleep immediately with no ACK;
the BLE connection drops.
- Silabs Flex: replies 0x0052, then closes the link and enters EM4.
- nRF: no response (deep sleep not supported).

Returns:
Command bytes: 0x0052 (2 bytes, big-endian)
"""
return CommandCode.DEEP_SLEEP.to_bytes(2, byteorder="big")


def build_direct_write_start_compressed(
uncompressed_size: int,
compressed_data: bytes,
Expand Down
27 changes: 25 additions & 2 deletions src/opendisplay/transport/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ def __init__(
self._client: BleakClient | None = None
self._notification_queue: asyncio.Queue[bytes] = asyncio.Queue()
self._notification_characteristic: BleakGATTCharacteristic | None = None
# Whether the command characteristic advertises Write Without Response.
# Set during notification setup; used to safely enable/disable WNR writes.
self._write_no_response_supported: bool = False
self.device_name: str | None = None

async def __aenter__(self) -> BLEConnection:
Expand Down Expand Up @@ -231,6 +234,16 @@ async def _setup_notifications(self) -> None:

self._notification_characteristic = characteristics[0]

# Record whether the characteristic advertises Write Without Response so
# writes can opt into it (0x71 data chunks) and gracefully fall back to
# write-with-response on devices/stacks that don't support it.
props = getattr(self._notification_characteristic, "properties", []) or []
self._write_no_response_supported = "write-without-response" in props
_LOGGER.debug(
"Command characteristic write-without-response supported: %s",
self._write_no_response_supported,
)

# Start notifications
await self._client.start_notify(
self._notification_characteristic,
Expand Down Expand Up @@ -283,11 +296,17 @@ def drain_notifications(self) -> int:
_LOGGER.warning("Discarded %d stale notification(s) before command", dropped)
return dropped

async def write_command(self, data: bytes) -> None:
async def write_command(self, data: bytes, response: bool = True) -> None:
"""Write command to device.

Args:
data: Command bytes to write
response: If True, use a BLE Write Request and wait for the ATT write
confirmation. If False, use a Write Without Response (Write Command)
to skip the ATT round-trip — used for bulk 0x71 image-data chunks,
which are still flow-controlled by the application-layer ACK. Falls
back to a Write Request if the characteristic does not advertise
write-without-response.

Raises:
BLEConnectionError: If not connected or write fails
Expand All @@ -302,11 +321,15 @@ async def write_command(self, data: bytes) -> None:
# from a clean queue (see drain_notifications).
self.drain_notifications()

# Only skip the write confirmation when the caller opts out AND the
# characteristic actually supports it; otherwise keep write-with-response.
effective_response = response or not self._write_no_response_supported

try:
await self._client.write_gatt_char(
self._notification_characteristic,
data,
response=True, # Wait for write confirmation
response=effective_response,
)
except Exception as e:
raise BLEConnectionError(f"Write failed: {e}") from e
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/test_auth_server_proof.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class _FakeConn:
def __init__(self, responses: list[bytes]) -> None:
self._responses = responses

async def write_command(self, data: bytes) -> None:
async def write_command(self, data: bytes, response: bool = True) -> None:
pass

async def read_response(self, timeout: float) -> bytes:
Expand Down
Loading