Skip to content

Commit

Permalink
feat: speed up to processing bluez passive data (#221)
Browse files Browse the repository at this point in the history
  • Loading branch information
bdraco authored Aug 9, 2023
1 parent 71e6fdf commit 8e7432d
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 23 deletions.
2 changes: 2 additions & 0 deletions src/dbus_fast/message.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ cdef object LITTLE_ENDIAN
cdef object PROTOCOL_VERSION

cdef object MESSAGE_FLAG
cdef object MESSAGE_FLAG_NONE
cdef object MESSAGE_TYPE_METHOD_CALL

cdef get_signature_tree

Expand Down
23 changes: 21 additions & 2 deletions src/dbus_fast/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@

MESSAGE_FLAG = MessageFlag

MESSAGE_FLAG_NONE = MessageFlag.NONE
MESSAGE_TYPE_METHOD_CALL = MessageType.METHOD_CALL


class Message:
"""A class for sending and receiving messages through the
Expand Down Expand Up @@ -101,8 +104,8 @@ def __init__(
path: Optional[str] = None,
interface: Optional[str] = None,
member: Optional[str] = None,
message_type: MessageType = MessageType.METHOD_CALL,
flags: Union[MessageFlag, int] = MessageFlag.NONE,
message_type: MessageType = MESSAGE_TYPE_METHOD_CALL,
flags: Union[MessageFlag, int] = MESSAGE_FLAG_NONE,
error_name: Optional[Union[str, ErrorType]] = None,
reply_serial: int = 0,
sender: Optional[str] = None,
Expand Down Expand Up @@ -153,6 +156,22 @@ def __init__(
if not getattr(self, field):
raise InvalidMessageError(f"missing required field: {field}")

def __repr__(self) -> str:
"""Return a string representation of this message."""
return (
f"<Message {self.message_type.name} "
f"serial={self.serial} "
f"reply_serial={self.reply_serial} "
f"sender={self.sender} "
f"destination={self.destination} "
f"path={self.path} "
f"interface={self.interface} "
f"member={self.member} "
f"error_name={self.error_name} "
f"signature={self.signature} "
f"body={self.body}>"
)

@staticmethod
def new_error(
msg: "Message", error_name: Union[str, ErrorType], error_text: str
Expand Down
14 changes: 9 additions & 5 deletions src/dbus_fast/message_bus.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@ cdef object MessageFlag

cdef object MESSAGE_TYPE_CALL
cdef object MESSAGE_TYPE_SIGNAL
cdef object NO_REPLY_EXPECTED_VALUE
cdef object NO_REPLY_EXPECTED
cdef object BLOCK_UNEXPECTED_REPLY
cdef object assert_object_path_valid
cdef object assert_bus_name_valid

cdef _expects_reply(Message msg)


cdef class SendReply:

cdef object _bus
Expand All @@ -28,12 +30,12 @@ cdef class BaseMessageBus:
cdef public object _user_disconnect
cdef public object _method_return_handlers
cdef public object _serial
cdef public object _path_exports
cdef public cython.dict _path_exports
cdef public cython.list _user_message_handlers
cdef public object _name_owners
cdef public cython.dict _name_owners
cdef public object _bus_address
cdef public object _name_owner_match_rule
cdef public object _match_rules
cdef public cython.dict _match_rules
cdef public object _high_level_client_initialized
cdef public object _ProxyObject
cdef public object _machine_id
Expand All @@ -45,7 +47,9 @@ cdef class BaseMessageBus:
cpdef _process_message(self, Message msg)

@cython.locals(
methods=cython.list,
method=_Method,
interface=ServiceInterface
interface=ServiceInterface,
interfaces=cython.list,
)
cdef _find_message_handler(self, Message msg)
57 changes: 47 additions & 10 deletions src/dbus_fast/message_bus.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,34 @@

MESSAGE_TYPE_CALL = MessageType.METHOD_CALL
MESSAGE_TYPE_SIGNAL = MessageType.SIGNAL
NO_REPLY_EXPECTED_VALUE = MessageFlag.NO_REPLY_EXPECTED.value
NO_REPLY_EXPECTED = MessageFlag.NO_REPLY_EXPECTED
_LOGGER = logging.getLogger(__name__)


_Message = Message


def _expects_reply(msg: _Message) -> bool:
return not (msg.flags.value & NO_REPLY_EXPECTED_VALUE)
"""Whether a message expects a reply."""
return not (msg.flags & NO_REPLY_EXPECTED)


def _block_unexpected_reply(reply: _Message) -> None:
"""Block a reply if it's not expected.
Previously we silently ignored replies that were not expected, but this
lead to implementation errors that were hard to debug. Now we log a
debug message instead.
"""
_LOGGER.debug(
"Blocked attempt to send a reply from handler "
"that received a message with flag "
"MessageFlag.NO_REPLY_EXPECTED: %s",
reply,
)


BLOCK_UNEXPECTED_REPLY = _block_unexpected_reply


class SendReply:
Expand All @@ -50,8 +71,7 @@ def __enter__(self):
return self

def __call__(self, reply: Message) -> None:
if _expects_reply(self._msg):
self._bus.send(reply)
self._bus.send(reply)

def _exit(
self,
Expand Down Expand Up @@ -855,9 +875,19 @@ def _process_message(self, msg: _Message) -> None:
if msg.message_type is MESSAGE_TYPE_CALL:
if not handled:
handler = self._find_message_handler(msg)
if not _expects_reply(msg):
if handler:
handler(msg, BLOCK_UNEXPECTED_REPLY)
else:
_LOGGER.error(
'"%s.%s" with signature "%s" could not be found',
msg.interface,
msg.member,
msg.signature,
)
return

send_reply = SendReply(self, msg)

with send_reply:
if handler:
handler(msg, send_reply)
Expand Down Expand Up @@ -892,8 +922,11 @@ def _make_method_handler(
def _callback_method_handler(
msg: Message, send_reply: Callable[[Message], None]
) -> None:
result = method_fn(interface, *msg_body_to_args(msg))
if not _expects_reply(msg):
return
body, fds = fn_result_to_body(
method_fn(interface, *msg_body_to_args(msg)),
result,
signature_tree=out_signature_tree,
replace_fds=negotiate_unix_fd,
)
Expand All @@ -911,8 +944,8 @@ def _callback_method_handler(
return _callback_method_handler

def _find_message_handler(
self, msg
) -> Optional[Callable[[Message, Callable], None]]:
self, msg: _Message
) -> Optional[Callable[[Message, Callable[[Message], None]], None]]:
if (
msg.interface == "org.freedesktop.DBus.Introspectable"
and msg.member == "Introspect"
Expand All @@ -937,8 +970,12 @@ def _find_message_handler(

msg_path = msg.path
if msg_path:
for interface in self._path_exports.get(msg_path, []):
for method in ServiceInterface._get_methods(interface):
interfaces = self._path_exports.get(msg_path)
if not interfaces:
return None
for interface in interfaces:
methods = ServiceInterface._get_methods(interface)
for method in methods:
if method.disabled:
continue

Expand Down
12 changes: 7 additions & 5 deletions src/dbus_fast/service.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,19 @@

import cython

from .signature cimport SignatureTree


cdef class _Method:

cdef public object name
cdef public str name
cdef public object fn
cdef public object disabled
cdef public object introspection
cdef public object in_signature
cdef public object out_signature
cdef public object in_signature_tree
cdef public object out_signature_tree
cdef public str in_signature
cdef public str out_signature
cdef public SignatureTree in_signature_tree
cdef public SignatureTree out_signature_tree

cdef class ServiceInterface:

Expand Down
2 changes: 1 addition & 1 deletion src/dbus_fast/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@


class _Method:
def __init__(self, fn, name, disabled=False):
def __init__(self, fn, name: str, disabled=False):
in_signature = ""
out_signature = ""

Expand Down
1 change: 1 addition & 0 deletions tests/test_marshaller.py
Original file line number Diff line number Diff line change
Expand Up @@ -602,5 +602,6 @@ def test_unmarshall_bluez_passive_message():
unmarshaller = Unmarshaller(stream)
unmarshaller.unmarshall()
message = unmarshaller.message
assert "/org/bluez/hci0/dev_58_D3_49_E6_02_6E" in str(message)
unpacked = unpack_variants(message.body)
assert unpacked == ["/org/bluez/hci0/dev_58_D3_49_E6_02_6E"]

0 comments on commit 8e7432d

Please sign in to comment.