tinygo-org / bluetooth

Cross-platform Bluetooth API for Go and TinyGo. Supports Linux, macOS, Windows, and bare metal using Nordic SoftDevice or HCI
https://tinygo.org
Other
764 stars 138 forks source link

bug: NINAFW adaptors broken when acting as peripheral #300

Open deadprogram opened 4 weeks ago

deadprogram commented 4 weeks ago

Since commit 457af7571ab5e7c3a9338cdd901a3b23b3c302d0 is appears that the NINAFW adaptors are broken. They advertise without a problem, however on connecting to them they do not correctly respond. Instead they get into a death spiral of bad messages probably due to losing their place.

jsoriano commented 1 week ago

Hey @deadprogram,

I was trying to use this library with an arduino-nano33 (Arduino Nano 33 IoT), and adapter.Enable() already fails. I can confirm the issue starts happening on https://github.com/tinygo-org/bluetooth/commit/457af7571ab5e7c3a9338cdd901a3b23b3c302d0.

This line looks suspicious to me: https://github.com/tinygo-org/bluetooth/commit/457af7571ab5e7c3a9338cdd901a3b23b3c302d0#diff-1acd23db7ad68fc2a873aff592c71288609ca6c78895f3c3ac3249791a1e2ae8R19

c := sz + 4 - (sz % 4)

When sz is 4, c will be 8, is this intended? Could it be reading parts of the next event when sz is already a multiple of 4?

I have tried to modify this line so it keeps multiples of 4 as they are, and it seems to work better.

c := sz + (4 - (sz % 4)) % 4

What is the intention of this line? To get the closest multiple of 4, or the next one?

Would it make sense to revert to the previous code based on ReadByte()?

deadprogram commented 1 week ago

Hi @jsoriano thanks for the correction, I added it to PR #304 just now.