Skip to content

Bluetooth: Fix several issues related to initialization with BT_SETTINGS #14834

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

Merged
merged 2 commits into from
Mar 25, 2019

Conversation

jhedberg
Copy link
Member

These two patches fix several critical details when it comes to initializing the stack when BT_SETTINGS is used.

Fixes #14833

@jhedberg jhedberg added bug The issue is a bug, or the PR is fixing a bug area: Bluetooth labels Mar 22, 2019
@jhedberg jhedberg added this to the v1.14.0 milestone Mar 22, 2019
Copy link
Collaborator

@Vudentz Vudentz left a comment

Choose a reason for hiding this comment

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

LGTM

@jhedberg jhedberg force-pushed the bt_settings_init branch 3 times, most recently from 2acd8f8 to dcc09f1 Compare March 22, 2019 11:40
@codecov-io
Copy link

codecov-io commented Mar 22, 2019

Codecov Report

Merging #14834 into master will increase coverage by 0.03%.
The diff coverage is 53.84%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14834      +/-   ##
==========================================
+ Coverage   52.49%   52.52%   +0.03%     
==========================================
  Files         309      309              
  Lines       45056    45063       +7     
  Branches    10432    10434       +2     
==========================================
+ Hits        23651    23669      +18     
+ Misses      16592    16583       -9     
+ Partials     4813     4811       -2
Impacted Files Coverage Δ
subsys/bluetooth/host/conn.c 40.19% <0%> (-0.12%) ⬇️
subsys/bluetooth/host/hci_core.c 44.13% <63.63%> (-0.02%) ⬇️
subsys/testsuite/ztest/src/ztest.c 72.72% <0%> (+7.43%) ⬆️
subsys/testsuite/ztest/include/ztest_assert.h 77.77% <0%> (+38.88%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d35b66...e2c982c. Read the comment docs.

Johan Hedberg added 2 commits March 24, 2019 19:35
The treatment of the BT_DEV_READY flag was broken when used together
with BT_SETTINGS. The flag would get set even though the stack was
still in a partially initialized state. Even worse, for central role
the stack would potentially try to initiate passive scanning without
having an identity address.

Refactor the code that sets the BT_DEV_READY flag (among other
initialization) into a separate bt_finalize_init() helper function and
call it when the settings have been loaded. Also clarify the warning
message given to the user in case settings_load() needs to be called.

Signed-off-by: Johan Hedberg <[email protected]>
Several public APIs were not checking the BT_DEV_READY flag, which
could lead to hard-to-debug behavior, particularly when the stack
lacks an identity address. Add the appropriate checks to these APIs.

Signed-off-by: Johan Hedberg <[email protected]>
Copy link
Collaborator

@fnde-ot fnde-ot left a comment

Choose a reason for hiding this comment

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

Nice catch.

@jhedberg jhedberg merged commit f811e54 into zephyrproject-rtos:master Mar 25, 2019
@jhedberg jhedberg deleted the bt_settings_init branch March 25, 2019 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Bluetooth bug The issue is a bug, or the PR is fixing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bluetooth init procedure with BT_SETTINGS is not reliable
4 participants