Skip to content

Commit f69b94b

Browse files
committed
connections: fix checksum race condition
This fixes a race condition where the nus_handler() could be called multiple times before returning to send_block() when it is waiting for the checksum_ready event. By setting self.expected_checksum = -1 in the nus_handler() callback itself, we prevent the next call from assuming that the data is also a checksum. Issue: #16
1 parent 1b8a6ca commit f69b94b

File tree

2 files changed

+19
-13
lines changed

2 files changed

+19
-13
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77
## [Unreleased]
88
### Changed
99
- Print BLE download progress after chunk is complete instead of before.
10+
### Fixed
11+
- Fix occasional bad checksum warning when running program via BLE.
1012

1113
## [1.0.0-alpha.2] - 2021-04-08
1214
### Added

pybricksdev/connections.py

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -616,16 +616,17 @@ def line_handler(self, line):
616616
print(line.decode())
617617

618618
def nus_handler(self, sender, data):
619-
# If we are currently downloading a program, treat incoming data as checksum.
620-
if self.loading:
621-
if data[0] == self.expected_checksum:
622-
self.checksum_ready.set()
623-
self.logger.debug("Correct checksum: {0}".format(data[0]))
624-
else:
625-
self.logger.warning("Expected checksum {0} but got {1}".format(
626-
self.expected_checksum,
627-
data[0]
628-
))
619+
# If we are currently expecting a checksum, validate it and notify the waiter
620+
if self.expected_checksum != -1:
621+
checksum = data[0]
622+
if checksum != self.expected_checksum:
623+
raise RuntimeError(
624+
f"Expected checksum {self.expected_checksum} but got {checksum}"
625+
)
626+
627+
self.expected_checksum = -1
628+
self.checksum_ready.set()
629+
self.logger.debug(f"Correct checksum: {checksum}")
629630
return
630631

631632
# Store incoming data
@@ -699,9 +700,9 @@ def disconnected_handler(self, _: BleakClient):
699700
await self.client.start_notify(NUS_TX_UUID, self.nus_handler)
700701
await self.client.start_notify(PYBRICKS_UUID, self.pybricks_service_handler)
701702
self.connected = True
702-
except BaseException as ex:
703+
except:
703704
self.disconnect()
704-
raise ex
705+
raise
705706

706707
async def disconnect(self):
707708
if self.connected:
@@ -722,8 +723,11 @@ async def send_block(self, data):
722723
try:
723724
await self.client.write_gatt_char(NUS_RX_UUID, bytearray(data), False)
724725
await asyncio.wait_for(self.checksum_ready.wait(), timeout=0.5)
725-
finally:
726+
except:
727+
# normally self.expected_checksum = -1 will be called in nus_handler()
728+
# but if we timeout or something like that, we need to reset it here
726729
self.expected_checksum = -1
730+
raise
727731

728732
async def run(self, py_path, wait=True, print_output=True):
729733

0 commit comments

Comments
 (0)