From e63dc15ede2cca59b52e0335a627769bee263f40 Mon Sep 17 00:00:00 2001 From: Gilles Boccon-Gibod Date: Tue, 27 Jan 2026 09:39:22 -0800 Subject: [PATCH] fix handling of return parameters --- bumble/device.py | 60 +++++++++------- bumble/drivers/intel.py | 24 ++++--- bumble/drivers/rtk.py | 56 ++++++++++----- bumble/hci.py | 22 +++++- bumble/host.py | 105 ++++++++++++++++++---------- bumble/transport/android_netsim.py | 2 +- docs/mkdocs/src/platforms/zephyr.md | 3 +- tests/hci_test.py | 4 +- tests/host_test.py | 20 +++++- 9 files changed, 200 insertions(+), 96 deletions(-) diff --git a/bumble/device.py b/bumble/device.py index 7b1b826..f2aee7e 100644 --- a/bumble/device.py +++ b/bumble/device.py @@ -2763,24 +2763,39 @@ class Device(utils.CompositeEventEmitter): logger.warning(f'!!! Command {command.name} timed out') raise CommandTimeoutError() from error - async def send_sync_command( - self, command: hci.HCI_SyncCommand[_RP], check_status: bool = True - ) -> _RP: + async def send_sync_command(self, command: hci.HCI_SyncCommand[_RP]) -> _RP: ''' Send a synchronous command via the host. + If the `status` field of the response's `return_parameters` is not equal to + `SUCCESS` an exception is raised. + Params: command: the command to send. - check_status: If `True`, check the `status` field of the response's - `return_parameters` and raise and exception if not equal to `SUCCESS`. Returns: An instance of the return parameters class associated with the command class. ''' try: - return await self.host.send_sync_command( - command, check_status, self.command_timeout - ) + return await self.host.send_sync_command(command, self.command_timeout) + except asyncio.TimeoutError as error: + logger.warning(f'!!! Command {command.name} timed out') + raise CommandTimeoutError() from error + + async def send_sync_command_raw( + self, command: hci.HCI_SyncCommand[_RP] + ) -> hci.HCI_Command_Complete_Event[_RP]: + ''' + Send a synchronous command via the host without checking the response. + + Params: + command: the command to send. + + Returns: + An HCI_Command_Complete_Event instance. + ''' + try: + return await self.host.send_sync_command_raw(command, self.command_timeout) except asyncio.TimeoutError as error: logger.warning(f'!!! Command {command.name} timed out') raise CommandTimeoutError() from error @@ -2812,12 +2827,13 @@ class Device(utils.CompositeEventEmitter): await self.host.reset() # Try to get the public address from the controller - response = await self.host.send_sync_command( - hci.HCI_Read_BD_ADDR_Command(), check_status=False - ) - if response.status == hci.HCI_SUCCESS: + try: + response = await self.host.send_sync_command(hci.HCI_Read_BD_ADDR_Command()) logger.debug(color(f'BD_ADDR: {response.bd_addr}', 'yellow')) self.public_address = response.bd_addr + except hci.HCI_Error: + logger.debug('Controller has no public address') + pass # Instantiate the Key Store (we do this here rather than at __init__ time # because some Key Store implementations use the public address as a namespace) @@ -2954,27 +2970,23 @@ class Device(utils.CompositeEventEmitter): ) if self.classic_enabled: - await self.send_sync_command( - hci.HCI_Write_Local_Name_Command(local_name=self.name.encode('utf8')), - check_status=False, + await self.send_sync_command_raw( + hci.HCI_Write_Local_Name_Command(local_name=self.name.encode('utf8')) ) - await self.send_sync_command( + await self.send_sync_command_raw( hci.HCI_Write_Class_Of_Device_Command( class_of_device=self.class_of_device - ), - check_status=False, + ) ) - await self.send_sync_command( + await self.send_sync_command_raw( hci.HCI_Write_Simple_Pairing_Mode_Command( simple_pairing_mode=int(self.classic_ssp_enabled) - ), - check_status=False, + ) ) - await self.send_sync_command( + await self.send_sync_command_raw( hci.HCI_Write_Secure_Connections_Host_Support_Command( secure_connections_host_support=int(self.classic_sc_enabled) - ), - check_status=False, + ) ) await self.set_connectable(self.connectable) await self.set_discoverable(self.discoverable) diff --git a/bumble/drivers/intel.py b/bumble/drivers/intel.py index 71888b8..ccddd4f 100644 --- a/bumble/drivers/intel.py +++ b/bumble/drivers/intel.py @@ -663,10 +663,13 @@ class Driver(common.Driver): async def read_device_info(self) -> dict[ValueType, Any]: self.host.ready = True - response1 = await self.host.send_sync_command( - hci.HCI_Reset_Command(), check_status=False - ) - if response1.status not in (hci.HCI_UNKNOWN_HCI_COMMAND_ERROR, hci.HCI_SUCCESS): + response1 = await self.host.send_sync_command_raw(hci.HCI_Reset_Command()) + if not isinstance( + response1.return_parameters, hci.HCI_StatusReturnParameters + ) or response1.return_parameters.status not in ( + hci.HCI_UNKNOWN_HCI_COMMAND_ERROR, + hci.HCI_SUCCESS, + ): # When the controller is in operational mode, the response is a # successful response. # When the controller is in bootloader mode, @@ -676,13 +679,18 @@ class Driver(common.Driver): raise DriverError("unexpected HCI response") # Read the firmware version. - response2 = await self.host.send_sync_command( - HCI_Intel_Read_Version_Command(param0=0xFF), check_status=False + response2 = await self.host.send_sync_command_raw( + HCI_Intel_Read_Version_Command(param0=0xFF) ) - if response2.status != 0: # type: ignore + if ( + not isinstance( + response2.return_parameters, HCI_Intel_Read_Version_ReturnParameters + ) + or response2.return_parameters.status != 0 + ): raise DriverError("HCI_Intel_Read_Version_Command error") - tlvs = _parse_tlv(response2.tlv) # type: ignore + tlvs = _parse_tlv(response2.return_parameters.tlv) # type: ignore # Convert the list to a dict. That's Ok here because we only expect each type # to appear just once. diff --git a/bumble/drivers/rtk.py b/bumble/drivers/rtk.py index 03d03b7..24bb78b 100644 --- a/bumble/drivers/rtk.py +++ b/bumble/drivers/rtk.py @@ -534,11 +534,13 @@ class Driver(common.Driver): @staticmethod async def get_loaded_firmware_version(host: Host) -> int | None: - response1 = await host.send_sync_command( - HCI_RTK_Read_ROM_Version_Command(), check_status=False - ) - - if response1.status != hci.HCI_SUCCESS: + response1 = await host.send_sync_command_raw(HCI_RTK_Read_ROM_Version_Command()) + if ( + not isinstance( + response1.return_parameters, HCI_RTK_Read_ROM_Version_ReturnParameters + ) + or response1.return_parameters.status != hci.HCI_SUCCESS + ): return None response2 = await host.send_sync_command( @@ -559,13 +561,20 @@ class Driver(common.Driver): await host.send_sync_command(hci.HCI_Reset_Command()) host.ready = True - command = hci.HCI_Read_Local_Version_Information_Command() - response = await host.send_sync_command(command, check_status=False) - if response.status != hci.HCI_SUCCESS: + response = await host.send_sync_command_raw( + hci.HCI_Read_Local_Version_Information_Command() + ) + if ( + not isinstance( + response.return_parameters, + hci.HCI_Read_Local_Version_Information_ReturnParameters, + ) + or response.return_parameters.status != hci.HCI_SUCCESS + ): logger.error("failed to probe local version information") return None - local_version = response + local_version = response.return_parameters logger.debug( f"looking for a driver: 0x{local_version.lmp_subversion:04X} " @@ -641,15 +650,21 @@ class Driver(common.Driver): # TODO: load the firmware - async def download_for_rtl8723b(self): + async def download_for_rtl8723b(self) -> int | None: if self.driver_info.has_rom_version: - response1 = await self.host.send_sync_command( - HCI_RTK_Read_ROM_Version_Command(), check_status=False + response1 = await self.host.send_sync_command_raw( + HCI_RTK_Read_ROM_Version_Command() ) - if response1.status != hci.HCI_SUCCESS: + if ( + not isinstance( + response1.return_parameters, + HCI_RTK_Read_ROM_Version_ReturnParameters, + ) + or response1.return_parameters.status != hci.HCI_SUCCESS + ): logger.warning("can't get ROM version") return None - rom_version = response1.version + rom_version = response1.return_parameters.version logger.debug(f"ROM version before download: {rom_version:04X}") else: rom_version = 0 @@ -691,13 +706,18 @@ class Driver(common.Driver): logger.debug("download complete!") # Read the version again - response2 = await self.host.send_sync_command( - HCI_RTK_Read_ROM_Version_Command(), check_status=False + response2 = await self.host.send_sync_command_raw( + HCI_RTK_Read_ROM_Version_Command() ) - if response2.status != hci.HCI_SUCCESS: + if ( + not isinstance( + response2.return_parameters, HCI_RTK_Read_ROM_Version_ReturnParameters + ) + or response2.return_parameters.status != hci.HCI_SUCCESS + ): logger.warning("can't get ROM version") else: - rom_version = response2.version + rom_version = response2.return_parameters.version logger.debug(f"ROM version after download: {rom_version:02X}") return firmware.version diff --git a/bumble/hci.py b/bumble/hci.py index 7e38814..d6696e0 100644 --- a/bumble/hci.py +++ b/bumble/hci.py @@ -2601,6 +2601,21 @@ class HCI_GenericReturnParameters(HCI_ReturnParameters): class HCI_StatusReturnParameters(HCI_ReturnParameters): status: HCI_ErrorCode = field(metadata=HCI_ErrorCode.type_metadata(1)) + @classmethod + def from_parameters(cls, parameters: bytes) -> Self | HCI_StatusReturnParameters: + status = HCI_ErrorCode(parameters[0]) + + if status != HCI_ErrorCode.SUCCESS: + # Don't parse further, just return the status. + return HCI_StatusReturnParameters(status=status) + + return cls(**HCI_Object.dict_from_bytes(parameters, 0, cls.fields)) + + +@dataclasses.dataclass +class HCI_GenericStatusReturnParameters(HCI_StatusReturnParameters): + data: bytes = field(metadata=metadata('*')) + @dataclasses.dataclass class HCI_StatusAndAddressReturnParameters(HCI_StatusReturnParameters): @@ -7498,6 +7513,7 @@ class HCI_Command_Complete_Event(HCI_Event, Generic[_RP]): def from_parameters(cls, parameters: bytes) -> Self: event = cls(**HCI_Object.dict_from_bytes(parameters, 0, cls.fields)) event.parameters = parameters + return_parameters_bytes = parameters[3:] # Find the class for the matching command. subclass = HCI_Command.command_classes.get(event.command_opcode) @@ -7510,16 +7526,16 @@ class HCI_Command_Complete_Event(HCI_Event, Generic[_RP]): 'HCI Command Complete event with opcode for a class that is not' ' an HCI_SyncCommand subclass: ' f'opcode={event.command_opcode:#04x}, ' - f'type={type(subclass).__name__}' + f'type={subclass.__name__}' ) event.return_parameters = HCI_GenericReturnParameters( - data=event.return_parameters # type: ignore[arg-type] + data=return_parameters_bytes ) # type: ignore[assignment] return event # Parse the return parameters bytes into an object. event.return_parameters = subclass.parse_return_parameters( - event.return_parameters # type: ignore[arg-type] + return_parameters_bytes ) # type: ignore[assignment] return event diff --git a/bumble/host.py b/bumble/host.py index ed8ec7e..54f30bf 100644 --- a/bumble/host.py +++ b/bumble/host.py @@ -270,7 +270,12 @@ class Host(utils.EventEmitter): self.sco_links = {} # SCO links, by connection handle self.bigs = {} # BIG Handle to BIS Handles self.pending_command: hci.HCI_SyncCommand | hci.HCI_AsyncCommand | None = None - self.pending_response: asyncio.Future[Any] | None = None + self.pending_response: ( + asyncio.Future[ + hci.HCI_Command_Complete_Event | hci.HCI_Command_Status_Event + ] + | None + ) = None self.number_of_supported_advertising_sets = 0 self.maximum_advertising_data_length = 31 self.local_version: ( @@ -658,25 +663,35 @@ class Host(utils.EventEmitter): response_timeout: float | None = None, ) -> hci.HCI_Command_Complete_Event | hci.HCI_Command_Status_Event: # Wait until we can send (only one pending command at a time) - async with self.command_semaphore: - assert self.pending_command is None - assert self.pending_response is None + await self.command_semaphore.acquire() - # Create a future value to hold the eventual response - self.pending_response = asyncio.get_running_loop().create_future() - self.pending_command = command + # Create a future value to hold the eventual response + assert self.pending_command is None + assert self.pending_response is None + self.pending_response = asyncio.get_running_loop().create_future() + self.pending_command = command - try: - self.send_hci_packet(command) - return await asyncio.wait_for( - self.pending_response, timeout=response_timeout - ) - except Exception: - logger.exception(color("!!! Exception while sending command:", "red")) - raise - finally: - self.pending_command = None - self.pending_response = None + response: ( + hci.HCI_Command_Complete_Event | hci.HCI_Command_Status_Event | None + ) = None + try: + self.send_hci_packet(command) + response = await asyncio.wait_for( + self.pending_response, timeout=response_timeout + ) + return response + except Exception: + logger.exception(color("!!! Exception while sending command:", "red")) + raise + finally: + self.pending_command = None + self.pending_response = None + if ( + response is not None + and response.num_hci_command_packets + and self.command_semaphore.locked() + ): + self.command_semaphore.release() @overload async def send_command( @@ -729,30 +744,42 @@ class Host(utils.EventEmitter): return response async def send_sync_command( + self, command: hci.HCI_SyncCommand[_RP], response_timeout: float | None = None + ) -> _RP: + response = await self.send_sync_command_raw(command, response_timeout) + return_parameters = response.return_parameters + + # Check the return parameters's status + if isinstance(return_parameters, hci.HCI_StatusReturnParameters): + status = return_parameters.status + elif isinstance(return_parameters, hci.HCI_GenericReturnParameters): + # if the payload has at least one byte, assume the first byte is the status + if not return_parameters.data: + raise RuntimeError('no status byte in return parameters') + status = hci.HCI_ErrorCode(return_parameters.data[0]) + else: + raise RuntimeError( + f'unexpected return parameters type ({type(return_parameters)})' + ) + if status != hci.HCI_ErrorCode.SUCCESS: + logger.warning( + f'{command.name} failed ' f'({hci.HCI_Constant.error_name(status)})' + ) + raise hci.HCI_Error(status) + + return return_parameters + + async def send_sync_command_raw( self, command: hci.HCI_SyncCommand[_RP], - check_status: bool = True, response_timeout: float | None = None, - ) -> _RP: + ) -> hci.HCI_Command_Complete_Event[_RP]: response = await self._send_command(command, response_timeout) # Check that the response is of the expected type assert isinstance(response, hci.HCI_Command_Complete_Event) - return_parameters: _RP = response.return_parameters - assert isinstance(return_parameters, command.return_parameters_class) - # Check the return parameters if required - if check_status: - if isinstance(return_parameters, hci.HCI_StatusReturnParameters): - status = return_parameters.status - if status != hci.HCI_SUCCESS: - logger.warning( - f'{command.name} failed ' - f'({hci.HCI_Constant.error_name(status)})' - ) - raise hci.HCI_Error(status) - - return return_parameters + return response async def send_async_command( self, @@ -1001,6 +1028,8 @@ class Host(utils.EventEmitter): self.pending_response.set_result(event) else: logger.warning('!!! no pending response future to set') + if event.num_hci_command_packets and self.command_semaphore.locked(): + self.command_semaphore.release() ############################################################ # HCI handlers @@ -1012,7 +1041,13 @@ class Host(utils.EventEmitter): if event.command_opcode == 0: # This is used just for the Num_HCI_Command_Packets field, not related to # an actual command - logger.debug('no-command event') + logger.debug('no-command event for flow control') + + # Release the command semaphore if needed + if event.num_hci_command_packets and self.command_semaphore.locked(): + logger.debug('command complete event releasing semaphore') + self.command_semaphore.release() + return return self.on_command_processed(event) diff --git a/bumble/transport/android_netsim.py b/bumble/transport/android_netsim.py index 904c576..e53bd60 100644 --- a/bumble/transport/android_netsim.py +++ b/bumble/transport/android_netsim.py @@ -194,7 +194,7 @@ async def open_android_netsim_controller_transport( # We only accept BLUETOOTH if request.initial_info.chip.kind != ChipKind.BLUETOOTH: - logger.warning('Unsupported chip type') + logger.debug('Request for unsupported chip type') error = PacketResponse(error='Unsupported chip type') await self.context.write(error) # return diff --git a/docs/mkdocs/src/platforms/zephyr.md b/docs/mkdocs/src/platforms/zephyr.md index dea7064..f003d6f 100644 --- a/docs/mkdocs/src/platforms/zephyr.md +++ b/docs/mkdocs/src/platforms/zephyr.md @@ -42,8 +42,7 @@ response = await host.send_sync_command( handle_type=HCI_Write_Tx_Power_Level_Command.TX_POWER_HANDLE_TYPE_ADV, connection_handle=0, tx_power_level=-4, - ), - check_status=False + ) ) if response.status == HCI_SUCCESS: diff --git a/tests/hci_test.py b/tests/hci_test.py index 6014a76..1ff4516 100644 --- a/tests/hci_test.py +++ b/tests/hci_test.py @@ -218,9 +218,9 @@ def test_return_parameters() -> None: assert isinstance(params.status, utils.OpenIntEnum) params = hci.HCI_Read_BD_ADDR_Command.parse_return_parameters( - bytes.fromhex('3C001122334455') + bytes.fromhex('00001122334455') ) - assert params.status == hci.HCI_ErrorCode.ADVERTISING_TIMEOUT_ERROR + assert params.status == hci.HCI_ErrorCode.SUCCESS assert isinstance(params.status, utils.OpenIntEnum) assert isinstance(params.bd_addr, hci.Address) diff --git a/tests/host_test.py b/tests/host_test.py index 895e4ad..3926232 100644 --- a/tests/host_test.py +++ b/tests/host_test.py @@ -26,9 +26,11 @@ from bumble.controller import Controller from bumble.hci import ( HCI_AclDataPacket, HCI_Command_Complete_Event, + HCI_Disconnect_Command, HCI_Error, HCI_ErrorCode, HCI_Event, + HCI_GenericReturnParameters, HCI_Reset_Command, HCI_StatusReturnParameters, ) @@ -195,6 +197,7 @@ async def test_send_sync_command() -> None: ) host = Host(source, sink) + host.ready = True # Sync command with success response1 = await host.send_sync_command(HCI_Reset_Command()) @@ -212,6 +215,17 @@ async def test_send_sync_command() -> None: assert excinfo.value.error_code == error_response.return_parameters.status - # Sync command with error status should not raise when `check_status` is False - response2 = await host.send_sync_command(HCI_Reset_Command(), check_status=False) - assert response2.status == HCI_ErrorCode.COMMAND_DISALLOWED_ERROR + # Sync command with raw result + response2 = await host.send_sync_command_raw(HCI_Reset_Command()) + assert response2.return_parameters.status == HCI_ErrorCode.COMMAND_DISALLOWED_ERROR + + # Sync command with a command that's not an HCI_SyncCommand + # (here, for convenience, we use an HCI_AsyncCommand instance) + command = HCI_Disconnect_Command(connection_handle=0x1234, reason=0x13) + sink.response = HCI_Command_Complete_Event( + 1, + command.op_code, + HCI_GenericReturnParameters(data=bytes.fromhex("00112233")), + ) + response3 = await host.send_sync_command_raw(command) # type: ignore + assert isinstance(response3.return_parameters, HCI_GenericReturnParameters)