-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Deprecate BT_FIXED_APPKEY #25414
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
Deprecate BT_FIXED_APPKEY #25414
Conversation
|
The following west manifest projects have changed revision in this Pull Request:
✅ All manifest checks OK Note: This message is automatically posted and updated by the Manifest GitHub Action. |
CI InformationTo view the history of this post, click the 'edited' button above Inputs:Sources:sdk-nrf: PR head: 8c7d26bc0d6b7a466ba86e95e2ea77f6f643e21b more detailssdk-nrf:
matter:
zephyr:
Github labels
List of changed files detected by CI (26)Outputs:ToolchainVersion: Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped;
|
|
You can find the documentation preview for this PR here. Preview links for modified nRF Connect SDK documents: https://ncsdoc.z6.web.core.windows.net/PR-25414/nrf/libraries/bluetooth/mesh/vnd/le_pair_resp.html |
PavelVPV
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think smp_bt_auth.c for sure should not use the deprecated API and should only be compiled with CONFIG_BT_APP_PASSKEY.
But also I think that since the LE Pairing Responder model is experimental, it is OK to change it is behavior and API without deprecation period. But we need to mention this change in the release notes.
Also, remember to update documentation of the model: https://github.com/nrfconnect/sdk-nrf/blob/main/doc/nrf/libraries/bluetooth/mesh/vnd/le_pair_resp.rst.
2999999 to
0d6a557
Compare
m-alperen-sener
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the documentation and added release notes for DFU distributer sample and LE Pair Responder model.
Remove all dependency on deprecated FIXED_PASSKEY, solely it depends on APP_PASSKEY now.
Removed immediate invalidation of passkey.
Update the Documentation diagram.
added commit to update matter door lock sample.
ArekBalysNordic
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some changes are required in the Matter Lock sample.
doc/nrf/releases_and_maturity/releases/release-notes-changelog.rst
Outdated
Show resolved
Hide resolved
0d6a557 to
d30f085
Compare
doc/nrf/releases_and_maturity/releases/release-notes-changelog.rst
Outdated
Show resolved
Hide resolved
| This function can be called from callbacks :c:member:`bt_conn_auth_info_cb.pairing_complete` and :c:member:`bt_conn_auth_info_cb.pairing_failed`. | ||
| See the :file:`samples/bluetooth/mesh/common/smp_bt_auth.c` file for the reference. | ||
| This model requires an application to only enable the display capability for the LE pairing by setting the :c:member:`bt_conn_auth_cb.pairing_display` callback. The application must also set the :c:member:`bt_conn_auth_cb.app_passkey` callback to use the passkey generated by LE Pairing Responder model. | ||
| After every pairing request, the application must invalidate the previously used passkey by calling the :c:func:`bt_mesh_le_pair_resp_passkey_invalidate` function or calling :c:func:`bt_mesh_le_pair_resp_passkey_set` with :ref:`BT_PASSKEY_RAND` value. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| After every pairing request, the application must invalidate the previously used passkey by calling the :c:func:`bt_mesh_le_pair_resp_passkey_invalidate` function or calling :c:func:`bt_mesh_le_pair_resp_passkey_set` with :ref:`BT_PASSKEY_RAND` value. | |
| After every pairing request, the application must invalidate the previously used passkey by calling the :c:func:`bt_mesh_le_pair_resp_passkey_invalidate` function or calling :c:func:`bt_mesh_le_pair_resp_passkey_set` with the ``BT_PASSKEY_RAND`` value. |
Any specific reason for using ref for a value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps it should be :c:macro:BT_PASSKEY_RAND?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not able to check this locally, does :c:macro: works with zephyr defined ones? I mean can doc pick up from zephyr?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do use it at least here and it works:
| The last element in the opcode list is always the :c:macro:`BT_MESH_MODEL_OP_END` macro: |
https://docs.nordicsemi.com/bundle/ncs-latest/page/nrf/protocols/bt/bt_mesh/vendor_model/dev_overview.html#adding_the_model_to_the_node_composition_data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, we can do that then 👍
d30f085 to
27124bd
Compare
|
Hi @m-alperen-sener, before merging could you check these things to be sure these changes still work:
mobile app is possible to check with mesh dfu distributor sample |
| * The :kconfig:option:`CONFIG_BT_FIXED_PASSKEY` kconfig option has been deprecated, replace it with the :kconfig:option:`CONFIG_BT_APP_PASSKEY` kconfig option. | ||
| Now, if you want to use a fixed passkey for the Matter Lock NUS service, register the ``app_passkey`` callback in the ``bt_conn_auth_cb`` structure. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * The :kconfig:option:`CONFIG_BT_FIXED_PASSKEY` kconfig option has been deprecated, replace it with the :kconfig:option:`CONFIG_BT_APP_PASSKEY` kconfig option. | |
| Now, if you want to use a fixed passkey for the Matter Lock NUS service, register the ``app_passkey`` callback in the ``bt_conn_auth_cb`` structure. | |
| * The :kconfig:option:`CONFIG_BT_FIXED_PASSKEY` Kconfig option has been deprecated, replace it with the :kconfig:option:`CONFIG_BT_APP_PASSKEY` Kconfig option. | |
| Now, if you want to use a fixed passkey for the Matter Lock NUS service, register the :c:member:`bt_conn_auth_cb.app_passkey` callback in the :c:struct:`bt_conn_auth_cb` structure. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@m-alperen-sener could you please add these changes to my PR, or add a new one with the changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| @@ -166,6 +166,10 @@ Bluetooth Mesh | |||
| * Deprecated the :kconfig:option:`CONFIG_BT_MESH_NLC_PERF_CONF` and :kconfig:option:`CONFIG_BT_MESH_NLC_PERF_DEFAULT` Kconfig options. | |||
| Existing configurations continue to work but you should migrate to individual profile options. | |||
|
|
|||
| * Update the LE Pairing Responder model: | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * Update the LE Pairing Responder model: | |
| * Updated the LE Pairing Responder model: |
bfe6ea8 to
215f1ea
Compare
m-alperen-sener
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DOC fix
Rebased on sdk-nrf main
Tried the IOS app; it triggers pairing succesfully with le pair responder. And SMP is functional. |
| @@ -43,11 +43,24 @@ void bt_mesh_le_pair_resp_passkey_invalidate(void); | |||
| * By default, passkeys will be randomly generated on every new request. This function allows to use | |||
| * pre-defined passkey instead. | |||
| * | |||
| * @params passkey Passkey to use for the pairing, or @ref BT_PASSKEY_INVALID to use randomly | |||
| * @params passkey Passkey to use for the pairing, or @ref BT_PASSKEY_RAND to use randomly | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The doc build error is because of reference to this macro. The macro is defined in the Zephyr PR. This will work only after the Zephyr PR - https://github.com/nrfconnect/sdk-zephyr/pull/3466/files#diff-4538e90f4e61dd34802aa1c0a5382dc89d4df6032ec6a3dbbf4eef746ebdd488 is merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good to know, then I can request merge for zephyr PR in spite of error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have removed @ref completely
215f1ea to
7b54a0f
Compare
|
Updated the image to fix doc fails, rebased to fix downstream failing tests. |
12fca63 to
0d56114
Compare
8308681 to
8ecddc4
Compare
810b80f to
1c0d944
Compare
| @@ -32,22 +32,35 @@ extern "C" { | |||
| _bt_mesh_le_pair_resp_op, NULL, NULL, \ | |||
| &_bt_mesh_le_pair_resp_cb) | |||
|
|
|||
| /* @brief Invalidate previously used passkey. | |||
| /** @brief Invalidate previously used passkey. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is wrong, the top line of a multiline comment should be empty e.g. /** then the text begins on the second line, will allow as this is not adding it just changing the indent but take note for future
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that was request from Carles I am confused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://www.doxygen.nl/manual/docblocks.html
It sound like @brief is allowed to be in the first line, if it is a base text it should start on the second line. I have check other documentation and it is aligned with that.
Manifest update to depracate and replace BT_FIXED_PADDKEY Signed-off-by: alperen sener <alperen.sener@nordicsemi.no>
BT_FIXED_PASSKEY is depracated so wee need to aling the le pair responder model with the new Kconfig BT_APP_PASSKEY usage. Adding bt_mesh_le_pair_resp_passkey_get to be able to retreive the passkey set randomly or by app. Signed-off-by: alperen sener <alperen.sener@nordicsemi.no>
BT_FIXED_PASSKEY is deprecated, thus we start using BT_APP_PASSKEY Signed-off-by: alperen sener <alperen.sener@nordicsemi.no>
CONFIG_BT_FIXED_PASSKEY has been deprecated. We switched to CONFIG_BT_APP_PASSKEY and added a migration guide entry. Signed-off-by: Arkadiusz Balys <arkadiusz.balys@nordicsemi.no>
1c0d944 to
8c7d26b
Compare
Deprecating BT_FIXED_PASSKEY and start using BT_APP_PASSKEY instead.