Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Two bug fixes #93

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

tobiasfunke1
Copy link

Hi,

first of all thank you for this great tool! I used it in one of my projects and it helps me to find a bug in a BLE smart lock.

While using your tool as a library and wrapping it with my code, I found two small bugs:

  1. missing method argument
  2. missing slice boundary

The first can be validated by looking at the implementation of ConnectionSniffer. In the method on_packet_received the method on_connection is called with a given timeout (packet.timeout):

self.access_address = packet.access_address
self.on_connection(
packet.inita,
packet.adva,
packet.crc_init,
packet.hop_interval,
packet.hop_increment,
packet.channel_map,
packet.timeout
)
self.state = self.STATE_FOLLOWING

I know the on_connection function is a placeholder and should be overridden like in CLIConnectionSniffer anyway, but I think the interface should be the same. So I would add the timeout argument here:

def on_connection(self, inita, adva, crc_init, interval, increment, channel_map):
print('Got connection !')

The second is an error during unpacking. Unpacking with unpack('<H', raw[1:])[0] of more than 2 bytes leads to struct.error: unpack requires a buffer of 2 bytes:

@staticmethod
def from_bytes(raw):
handle = unpack('<H', raw[1:])[0]
value = raw[3:]
return WriteRequest(handle, value)

For the WriteCommand it was sliced correctly:

@staticmethod
def from_bytes(raw):
handle = unpack('<H', raw[1:3])[0]
value = raw[3:]
return WriteRequest(handle, value)

I assume that this behavior also occurs with other from_bytes implementations in the same module like ExchangeMtuResponse, ExchangeMtuRequest and ErrorResponse, but I am not 100% sure. Maybe some can confirm that?

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