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

fix: correct length being returned and used for BLE HCI reads #59

Merged
merged 1 commit into from
Feb 21, 2025

Conversation

deadprogram
Copy link
Contributor

This PR is to address #58 along with tinygo-org/bluetooth#301 and tinygo-org/bluetooth#327

Basically, from looking at the data, I realized that the length being returned was exactly one less then the same packet from the ninafw HCI. Bumping that up allows the code to run again as expected.

@soypat
Copy link
Owner

soypat commented Feb 21, 2025

The change is quite counterintuitive, it seems like a fix that might break in the future for packets of varying lengths since we are reading at least length+4 but returning length+1. Here's a couple of suggestions:

  • Move the change to a single line, instead of returning length+1 in several places do length++ on a line before it
  • You did bring an apparent mistake to my attentions, we read rounded(length+4)=roundedLength bytes from the ring buffer but return length! Maybe the reference returns the correct value here (overlooked on my part). Have you tried returning hciLength or roundedLength And seeing if it works?
  • Try just adding 1 instead of 4 to the value of length to get hciLength and then repeat above step

@deadprogram
Copy link
Contributor Author

Have you tried returning hciLength or roundedLength And seeing if it works?

Tried hciLength and that worked.

Try just adding 1 instead of 4 to the value of length to get hciLength and then repeat above step

That did NOT work.

So I will change to using hciLength since it is in fact a bit more intuitive, and did work in all my testing.

@deadprogram deadprogram force-pushed the correct-hci-read-length branch from 51d69fd to 7c14e54 Compare February 21, 2025 11:22
@soypat soypat merged commit d06a11e into soypat:main Feb 21, 2025
@soypat
Copy link
Owner

soypat commented Feb 21, 2025

Thank you a million ron!

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.

2 participants