Fixed eddystone_scanner fallback issue (New)#1967
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1967 +/- ##
==========================================
+ Coverage 50.60% 50.88% +0.27%
==========================================
Files 384 385 +1
Lines 41180 41437 +257
Branches 7642 7701 +59
==========================================
+ Hits 20841 21086 +245
- Misses 19589 19593 +4
- Partials 750 758 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ce8261a to
60716d7
Compare
fixed eddystone_scanner fallback issue
revicsed scripts and unit tests
update unit tests
revised unit test
revised unit test
revised scripts and unit tests
revised unit test
revised unit tests to fit python3.5
cf595b9 to
0d7588c
Compare
revised scanner.py
update the index of the command status code
seankingyang
left a comment
There was a problem hiding this comment.
Just some tiny thing need to modify.
Log the Adv event with the string not the hex will be friendly for others to know about this.
checkbox-support/checkbox_support/vendor/beacontools/scanner.py
Outdated
Show resolved
Hide resolved
checkbox-support/checkbox_support/vendor/beacontools/scanner.py
Outdated
Show resolved
Hide resolved
revised scripts and unit tests
|
@stanley31huang We have very old systems (like Tilamook UC16) that are using this test still. Could you make sure these won't need the vendorized scripts that are removed in this PR? |
I will verify the changes in Tillamook UC16 and paste the test results later. |
|
tested on Tillamook running UC16 and updated the test results in the description field. |
|
I am not sure why we need the scanning thing to be ran on another thread when nothing else is running on the main thread, would be glad to see if you could help to eliminate that. |
|
The test result of this PR on realtek 8852be is bad, the test only passes 32 times out of 100. |
|
A few comments about this:
(I'll leave you to @fernando79513 review which will be more comprehensive, I decided to give my opinion given what @pseudocc wrote. I don't think that is constructive) |
|
@Hook25 The beacontools library is outdated, with its latest release in 2020. I’ve made some improvements for BT 5.4+ and added a check for the LE Meta event type before parsing the raw event data. Also, I’d like to point out that the code provided by @pseudocc is designed to handle only LE advertising reports, not LE extended advertising reports |
|
@Hook25 Will report an issue next time. |
|
I agree with @Hook25 that modifying the vendorized library by ourselves could cause some unexpected issues, but if the beacontools library is no longer maintained, I don't think we have another alternative. Apart from that, I dug into the checkobx history and I found the PR in which they created the separation between both beacontools versions. |
|
@fernando79513 I tested both dawson platform and it works well, please see the details from the description field. |
fernando79513
left a comment
There was a problem hiding this comment.
Thank you very much for all the changes and for testing them on several devices. Overall, I think we can go on with these changes.
I just added a few comments related to the logging.
checkbox-support/checkbox_support/vendor/beacontools/scanner.py
Outdated
Show resolved
Hide resolved
checkbox-support/checkbox_support/vendor/beacontools/scanner.py
Outdated
Show resolved
Hide resolved
checkbox-support/checkbox_support/vendor/beacontools/scanner.py
Outdated
Show resolved
Hide resolved
checkbox-support/checkbox_support/vendor/beacontools/scanner.py
Outdated
Show resolved
Hide resolved
checkbox-support/checkbox_support/vendor/beacontools/scanner.py
Outdated
Show resolved
Hide resolved
checkbox-support/checkbox_support/vendor/beacontools/scanner.py
Outdated
Show resolved
Hide resolved
checkbox-support/checkbox_support/vendor/beacontools/scanner.py
Outdated
Show resolved
Hide resolved
checkbox-support/checkbox_support/vendor/beacontools/scanner.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com>
Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com>
Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com>
Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com>
Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com>
Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com>
Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com>
Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com>
Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com>
Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com>
update logging function and unittest
5bc283b to
9e23973
Compare
|
Thanks for all of suggestions raised by @fernando79513, all of patches has been applied and the unit test has been updated. here is the results tested on a DUT with latest patch |
fernando79513
left a comment
There was a problem hiding this comment.
TYVM for the great work here.
I think we are ready to merge it.
* Fixed eddystone_scanner fallback issue fixed eddystone_scanner fallback issue * revised scripts and unit tests revicsed scripts and unit tests * update unit tests update unit tests * revised unit test revised unit test * revised unit test revised unit test * revised scripts and unit tests revised scripts and unit tests * revised unit test revised unit test * revised unit tests to fit python3.5 revised unit tests to fit python3.5 * revised scanner.py revised scanner.py * updated command status code update the index of the command status code * Revised scripts and unit tests revised scripts and unit tests * Update checkbox-support/checkbox_support/vendor/beacontools/scanner.py Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com> * Update checkbox-support/checkbox_support/vendor/beacontools/scanner.py Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com> * Update checkbox-support/checkbox_support/vendor/beacontools/scanner.py Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com> * Update checkbox-support/checkbox_support/vendor/beacontools/scanner.py Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com> * Update checkbox-support/checkbox_support/vendor/beacontools/scanner.py Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com> * Update checkbox-support/checkbox_support/vendor/beacontools/scanner.py Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com> * Update checkbox-support/checkbox_support/vendor/beacontools/scanner.py Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com> * Update checkbox-support/checkbox_support/vendor/beacontools/scanner.py Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com> * Update checkbox-support/checkbox_support/vendor/beacontools/scanner.py Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com> * Update checkbox-support/checkbox_support/vendor/beacontools/scanner.py Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com> * update logging function and unittest update logging function and unittest --------- Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com>
* Fixed eddystone_scanner fallback issue fixed eddystone_scanner fallback issue * revised scripts and unit tests revicsed scripts and unit tests * update unit tests update unit tests * revised unit test revised unit test * revised unit test revised unit test * revised scripts and unit tests revised scripts and unit tests * revised unit test revised unit test * revised unit tests to fit python3.5 revised unit tests to fit python3.5 * revised scanner.py revised scanner.py * updated command status code update the index of the command status code * Revised scripts and unit tests revised scripts and unit tests * Update checkbox-support/checkbox_support/vendor/beacontools/scanner.py Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com> * Update checkbox-support/checkbox_support/vendor/beacontools/scanner.py Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com> * Update checkbox-support/checkbox_support/vendor/beacontools/scanner.py Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com> * Update checkbox-support/checkbox_support/vendor/beacontools/scanner.py Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com> * Update checkbox-support/checkbox_support/vendor/beacontools/scanner.py Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com> * Update checkbox-support/checkbox_support/vendor/beacontools/scanner.py Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com> * Update checkbox-support/checkbox_support/vendor/beacontools/scanner.py Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com> * Update checkbox-support/checkbox_support/vendor/beacontools/scanner.py Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com> * Update checkbox-support/checkbox_support/vendor/beacontools/scanner.py Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com> * Update checkbox-support/checkbox_support/vendor/beacontools/scanner.py Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com> * update logging function and unittest update logging function and unittest --------- Co-authored-by: Fernando Bravo <39527354+fernando79513@users.noreply.github.com>
fixed eddystone_scanner fallback issue
Description
In the
beacontools/scanner.py, it will issueLE Advertising Scan EnableorLE Extended Advertising Scan Enablecommand based on the HCI Core version, so we should not have the fallback mechanisim in the eddystone_scanner.py script.Also based on the bluetooth 6.0 specification, we should not issue the legacy command if LE feature is supported
Resolved issues
Documentation
Tests
Sideloaded the checkbox-support modules on system running Ubuntu Desktop 18 with bluetooth 4.2 adapter
On an IoT platform running Ubuntu Server 24 with bluetooth 5.4
On an IoT platform running Ubuntu Server 24 with bluetooth 6.0 adapter
On Tillamook running UC16
BTW, I tested this changes with hci0 interface only, due to the other hci interface been excluded in the launcher
https://git.launchpad.net/~tillamook-team/tillamook/+git/checkbox-tillamook/tree/checkbox-provider-tillamook/units/test-plans.pxu#n215
On Dawson-001(i) running UC20
On Dawson-004(j) running UC20