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

esp32_ble: Consider ESP_BT_STATUS_DONE a successful state #6493

Merged
merged 3 commits into from Apr 22, 2024

Conversation

polyfloyd
Copy link
Contributor

@polyfloyd polyfloyd commented Apr 7, 2024

What does this implement/fix?

This fixes the log messages below from repeatedly being written:

[00:23:57][E][esp32_ble_tracker:194]: Scan set param failed: 5
[00:23:57][D][esp32_ble_tracker:266]: Starting scan...
[00:23:57][D][esp32_ble_tracker:186]: Stopping scan after failure...

The issue seems to arise from BT_STATE_DONE being interpreted as an error. Following the documentation for this value leads to packages/framework-espidf/components/bt/common/include/bt_common.h describing it as request already completed. Assuming that whatever was being performed is idempotent, mapping this state to success should be ok.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)

Related issue: esphome/issues#4058

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040
  • BK72xx
  • RTL87xx

Example entry for config.yaml:

esp32_ble_tracker:
  id: ble_tracker
  scan_parameters:
    interval: 20s
    window: 10s
    duration: 1min
    active: false
    continuous: true

sensor:
  - platform: xiaomi_hhccjcy01
    mac_address: 'FF:FF:FF:FF:FF:FF'
    temperature:
      name: "Miflora 01 Temperature"
    moisture:
      name: "Miflora 01 Moisture"
    illuminance:
      name: "Miflora 01 Illuminance"
    conductivity:
      name: "Miflora 01 Soil Conductivity"

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

@codecov-commenter
Copy link

codecov-commenter commented Apr 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 53.43%. Comparing base (4d8b5ed) to head (b9fe073).
Report is 424 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #6493      +/-   ##
==========================================
- Coverage   53.70%   53.43%   -0.28%     
==========================================
  Files          50       50              
  Lines        9408     9537     +129     
  Branches     1654     1685      +31     
==========================================
+ Hits         5053     5096      +43     
- Misses       4056     4130      +74     
- Partials      299      311      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@polyfloyd
Copy link
Contributor Author

Sooo, anything else that needs to happen? I consider this ready to be reviewed

@polyfloyd polyfloyd changed the title esp32_ble_tracker: Consider ESP_BT_STATUS_DONE a successful state esp32_ble: Consider ESP_BT_STATUS_DONE a successful state Apr 19, 2024
@polyfloyd
Copy link
Contributor Author

@clydebarrow Could you please have a glance?

Co-authored-by: Clyde Stubbs <2366188+clydebarrow@users.noreply.github.com>
Copy link
Contributor

@clydebarrow clydebarrow left a comment

Choose a reason for hiding this comment

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

LGTM

@polyfloyd
Copy link
Contributor Author

Thanks!

@jesserockz jesserockz added this to the 2024.4.1 milestone Apr 22, 2024
@jesserockz jesserockz merged commit aee2a49 into esphome:dev Apr 22, 2024
98 checks passed
jesserockz pushed a commit that referenced this pull request Apr 22, 2024
Co-authored-by: Clyde Stubbs <2366188+clydebarrow@users.noreply.github.com>
@jesserockz jesserockz mentioned this pull request Apr 22, 2024
@polyfloyd polyfloyd deleted the fix-ble-log-spam branch April 23, 2024 12:35
@github-actions github-actions bot locked and limited conversation to collaborators Apr 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants