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

Added extended BLE commands. #54

Merged
merged 2 commits into from
Oct 26, 2020
Merged

Conversation

idaniel86
Copy link
Contributor

Hi guys, there is an issue with legacy BLE commands using bluez with version 5 and probably higher. Commands like OCF_LE_SET_SCAN_PARAMETERS and OCF_LE_SET_SCAN_ENABLE return error COMMAND_DISALLOWED. After reading few discussions this is due to commands being deprecated and extended version of these commands should be used. I have added check for HCI version and based on this value use legacy or extended command versions. I had this issue on Ubuntu 18.04 and 19.10. When reverting the Ubuntu 18.04 kernel to the original one, the legacy commands worked. My update made beacontools to work with Ubuntu 19.10 and bluez 5.50. Feel free to rework the changes I proposed.

@coveralls
Copy link

coveralls commented Oct 5, 2020

Coverage Status

Coverage decreased (-1.5%) to 86.456% when pulling 9d5948a on idaniel86:ext-adv_linux into 360b1ef on citruz:master.

Copy link
Owner

@citruz citruz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pull request! This looks really important to make beacontools future proof.
Please see my comments for some changes that we should do before merging to avoid code duplication.
There are also some linting errors. See https://travis-ci.org/github/citruz/beacontools/jobs/732906931

beacontools/scanner.py Outdated Show resolved Hide resolved
beacontools/scanner.py Show resolved Hide resolved
beacontools/scanner.py Outdated Show resolved Hide resolved
@idaniel86
Copy link
Contributor Author

Merged the commands and added descriptions. However I am getting still a stupid lint error "R0913: Too many arguments (7/5) (too-many-arguments)" in the helper hci_send_req function that just calls the bluez hci_send_req function and passes down the arguments.

BTW: An important issue, I have implemented the hci_send_req only for linux backend! Someone with knowledge how to implement it under freebsd and has an environment for testing has to implement it.

@citruz
Copy link
Owner

citruz commented Oct 25, 2020

Sorry for the delay. I now have a setup to test your PR and will hopefully be able to look at it in the next week.
Regarding FreeBSD, maybe @myfreeweb can help?

@citruz citruz changed the base branch from master to release2.1 October 26, 2020 21:21
@citruz citruz merged commit c11382c into citruz:release2.1 Oct 26, 2020
@citruz
Copy link
Owner

citruz commented Oct 26, 2020

Awesome! Except for a small typo, your PR worked out of the box on my Ubuntu 20.04 test machine.
I added a NotImplementedError for send_req in the FreeBSD backend, so that it falls back to the legacy handling automatically. In that way we don't break the FreeBSD support completely.
I will finish the release in the next days. Thanks again for the contribution 🚀

@citruz
Copy link
Owner

citruz commented Oct 26, 2020

(This should also fix #46)

@idaniel86
Copy link
Contributor Author

Perfect!!! I am glad I could be of help.

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.

3 participants