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 for issue #4158: BLEAdvertising - Crash with stack trace originating in Bluedroid #4182

Merged
merged 4 commits into from
Nov 2, 2020

Conversation

esikora
Copy link
Contributor

@esikora esikora commented Jul 18, 2020

Fixed Issue:

#4158

Summary of the Problem:

After calling BLEAdvertising::start(), a crash with a stack trace originating in Bluedroid occurs.
This crash occurs when the payload size of the scan response message exceeds 31 bytes.
Apparently, Bluedroid does not account for the attributes 'Flags' and 'Appearance' being set in the scan response data.

Solution:

Improved configuration of scan response data in BLEAdvertising::start() avoids the crash:

  • Added a new member variable m_scanRespData to be able to configure the scan response data differently from the advertising data.
  • Initialization of m_scanRespData in BLEAdvertising constructor.
  • Use of m_scanRespData within BLEAdvertising::start() to configure the scan response data.
  • Within BLEAdvertising::start(), the content of m_advData is copied into m_scanRespData, and then 'Flags' and 'Appearance' are cleared in m_scanRespData.

Result:

With this fix, device names of up to 29 characters can be used without causing a crash.

Further Considerations:

  • Behaviour of BLE library changed as little as possible. Only the two attributes that caused problems are omitted in the scan response data. They are still contained in the advertisement data.
  • Lifetime of m_scanRespData accounts for the remark in the documentation of esp_ble_gap_config_adv_data(): 'This memory space cannot be freed until callback of config_adv_data is received'.
  • Tested that the configuration of ServiceUUIDs in the scan response data does not cause crashes. Thus, these do not need to be set to '0' / 'nullptr' in the scan response struct.

Not Solved (Future Work):

  • A device name with more than 29 characters still causes a crash if scan response is enabled. Checking the length is not possible in 'BLEAdvertising'. It could be done in 'BLEDevice'.
  • I think, that the line delete[] m_advData.p_service_uuid; in BLEAdvertising::start() might violate the condition to wait for the callback of esp_ble_gap_config_adv_data (see above). Yet, I did not experience problems related to this. Hence, I did not change this code.

…luedroid

Improved configuration of scan response data in 'BLEAdvertising' avoids the crash:
- Added member variable 'm_scanRespData' to configure scan response differently from advertising data
- Initialization of 'm_scanRespData' in BLEAdvertising constructor
- Use of 'm_scanRespData' within BLEAdvertising::start() to configure the scan response
- 'Flags' and 'Appearance' are cleared in the scan response data
- With this fix, device names of up to 29 characters can be used without causing a crash.
@esikora esikora changed the title Fix for issue #4158: Crash with stack trace originating in Bluedroid Fix for issue #4158: BLEAdvertising - Crash with stack trace originating in Bluedroid Jul 18, 2020
@me-no-dev me-no-dev merged commit 7c05721 into espressif:master Nov 2, 2020
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