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

Bluetooth: Mesh: Move dfu mbt models into their own elements #81531

Conversation

m-alperen-sener
Copy link
Collaborator

@m-alperen-sener m-alperen-sener commented Nov 18, 2024

  • Tests: Bluetooth: Tester: Mesh DFUM and BLOB should have own elements
    Moving the blob client, dfd server and dfu server to their own elements and increasing the CONFIG_BT_MESH_TX_SEG_MAX to 8 to fit new composition data into composition data page status message.
    Standalone mesh blob client and DFU distributor/update server models requires one element and those elements only contain the main models and the models they extend to.
    Referring to MshMBT_v1.0 Section 6.1: The BLOB Transfer Client model defines the messages listed in Table 6.1 , and requires one element: the BLOB Transfer Client Main element. The BLOB Transfer Client Main element contains the BLOB Transfer Client main model.
    And referring to MshDFU_v1.0 Sections 6.1.1 and 6.2.1:
    6.1.1
    The Firmware Update Server model adds the state instances listed in Table 6.1 and Table 6.2 and the messages listed in Table 6.3 to the model it extends, and requires one element: the Firmware Update Main element. The Firmware Update Main element contains the Firmware Update Server main model and all the models that the main model extends.
    6.2.1
    The Firmware Distribution Server model adds the state instances listed in Table 6.7 and Table 6.8 and the messages listed in Table 6.9 to the model it extends, and requires one element: the Firmware Distribution Main element. The Firmware Distribution Main element contains the Firmware Distribution Server main model and all the models that the main model extends.

  • Bluetooth: Mesh: Check that required models exists on the same element
    Referring to MshDFU_v1.0 Sections 6.1.1, 6.2.1 and 7.1.1 model descriptions: DFU/DFD server/clients extend BLOB Transfer root models and DFD server requires Firmware Update Client on the same element. For this reason we need to make sure that those main models or root models exist on the same element. And also firmware update client can not be forced to be in the first element.

  • For all model extention call return the error code in case of an error.

@zephyrbot zephyrbot added area: Bluetooth Qualification Bluetooth Qualification -related issues and pull requests area: Bluetooth Mesh area: Bluetooth labels Nov 18, 2024
m-alperen-sener added a commit to m-alperen-sener/auto-pts that referenced this pull request Nov 18, 2024
Alinging the bot for mesh blob client tests with zephyr
tester.

zephyrproject-rtos/zephyr#81531

Signed-off-by: alperen sener <[email protected]>
m-alperen-sener added a commit to m-alperen-sener/auto-pts that referenced this pull request Nov 18, 2024
Aligning the bot for mesh blob client tests with zephyr
tester.

zephyrproject-rtos/zephyr#81531

Signed-off-by: alperen sener <[email protected]>
@Thalley Thalley removed their request for review November 18, 2024 15:51
subsys/bluetooth/mesh/dfu_cli.c Outdated Show resolved Hide resolved
tests/bluetooth/tester/src/btp_mesh.c Outdated Show resolved Hide resolved
tests/bluetooth/tester/src/btp_mesh.c Show resolved Hide resolved
omkar3141
omkar3141 previously approved these changes Nov 19, 2024
Copy link
Collaborator

@omkar3141 omkar3141 left a comment

Choose a reason for hiding this comment

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

Looks fine to me, of course there are some old naming conventions issues, but it is up to you if you want to fix it.

@m-alperen-sener m-alperen-sener force-pushed the move_dfu_mbt_models_into_their_own_elements branch 3 times, most recently from 8267976 to 03eb447 Compare November 20, 2024 12:10
PavelVPV
PavelVPV previously approved these changes Nov 20, 2024
@PavelVPV PavelVPV changed the title Move dfu mbt models into their own elements Bluetooth: Mesh: Move dfu mbt models into their own elements Nov 20, 2024
@m-alperen-sener m-alperen-sener force-pushed the move_dfu_mbt_models_into_their_own_elements branch from 03eb447 to e18dadd Compare November 20, 2024 12:28
subsys/bluetooth/mesh/priv_beacon_srv.c Outdated Show resolved Hide resolved
subsys/bluetooth/mesh/dfu_srv.c Outdated Show resolved Hide resolved
@m-alperen-sener m-alperen-sener force-pushed the move_dfu_mbt_models_into_their_own_elements branch from e18dadd to b0ff4ee Compare November 20, 2024 13:05
Copy link
Collaborator

@alxelax alxelax left a comment

Choose a reason for hiding this comment

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

LGTM at the moment. When you satisfy the Compliance checker, I'll approve it.

@m-alperen-sener m-alperen-sener force-pushed the move_dfu_mbt_models_into_their_own_elements branch from b0ff4ee to 53303c3 Compare November 20, 2024 13:18
alxelax
alxelax previously approved these changes Nov 20, 2024
PavelVPV
PavelVPV previously approved these changes Nov 20, 2024
@m-alperen-sener m-alperen-sener force-pushed the move_dfu_mbt_models_into_their_own_elements branch from 53303c3 to b2323de Compare November 20, 2024 15:47
@PavelVPV
Copy link
Collaborator

@sjanc, could you please look at this PR?

subsys/bluetooth/mesh/dfd_srv.c Outdated Show resolved Hide resolved
subsys/bluetooth/mesh/dfd_srv.c Outdated Show resolved Hide resolved
subsys/bluetooth/mesh/dfu_cli.c Outdated Show resolved Hide resolved
subsys/bluetooth/mesh/dfu_srv.c Outdated Show resolved Hide resolved
@m-alperen-sener m-alperen-sener force-pushed the move_dfu_mbt_models_into_their_own_elements branch from b2323de to 659fdc7 Compare November 27, 2024 15:31
@m-alperen-sener
Copy link
Collaborator Author

Tried to shorten error log messages without losing explanation for what the actual problem is. The key point is "not being on the same element", just saying "missing" might not give the message, or puzzle people. I am OK with removing all err logs anyway 👍

@m-alperen-sener m-alperen-sener force-pushed the move_dfu_mbt_models_into_their_own_elements branch from 659fdc7 to 39665bc Compare November 27, 2024 15:40
@omkar3141
Copy link
Collaborator

Tried to shorten error log messages without losing explanation for what the actual problem is. The key point is "not being on the same element", just saying "missing" might not give the message, or puzzle people. I am OK with removing all err logs anyway 👍

It is not necessary at all. If there is a problem, the log messages anyway print files names, etc. Even if MINIMAL is enabled, simply searching by string globally one can easily locate the log strings in code base. I feel, we shouldn't treat log messages as a way to educate users about what is the right thing to do. Instead, use them to actually print error codes or some useful information. This keeps application footprint low at development stage itself.

Referring to MshDFU_v1.0 Sections 6.1.1, 6.2.1 and  7.1.1 model
descriptions: DFU/DFD server/clients extend BLOB Transfer root models
and DFD server requires Firmware Update Client on the same element. For
this reason we need to make sure that those main models or root models
exist on the same element. And also firmware update client can not be
forced to be in the first element.

For all model extention call return the error code in case of an error.

Signed-off-by: alperen sener <[email protected]>
Moving the blob client, dfd server and dfu server to their own
elements and increasing the CONFIG_BT_MESH_TX_SEG_MAX to 8 to fit
new composition data into composition data page status message.

Standalone mesh blob client and DFU distributor/update server models
requires one element and those elements only contain the main models
and the models they extend to.

Referring to MshMBT_v1.0 Section 6.1:
The BLOB Transfer Client model defines the messages listed in Table 6.1
, and requires one element: the BLOB Transfer Client Main element. The
BLOB Transfer Client Main element contains the BLOB Transfer Client
main model.

And referring to MshDFU_v1.0 Sections 6.1.1 and 6.2.1:
6.1.1
The Firmware Update Server model adds the state instances listed in
Table 6.1 and Table 6.2 and the messages listed in Table 6.3 to the
model it extends, and requires one element: the Firmware Update Main
element. The Firmware Update Main element contains the Firmware Update
Server main model and all the models that the main model extends.
6.2.1
The Firmware Distribution Server model adds the state instances listed
in Table 6.7 and Table 6.8 and the messages listed in Table 6.9 to the
model it extends, and requires one element: the Firmware Distribution
Main element. The Firmware Distribution Main element contains the
Firmware Distribution Server main model and all the models that the
main model extends.

Signed-off-by: alperen sener <[email protected]>
@m-alperen-sener m-alperen-sener force-pushed the move_dfu_mbt_models_into_their_own_elements branch from 39665bc to ae56d8b Compare November 28, 2024 14:31
Copy link
Collaborator

@omkar3141 omkar3141 left a comment

Choose a reason for hiding this comment

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

Thanks. Looks good to me.

@kartben
Copy link
Collaborator

kartben commented Nov 29, 2024

@sjanc PTAL

@kartben kartben merged commit b78eea2 into zephyrproject-rtos:main Dec 4, 2024
26 checks passed
sjanc pushed a commit to auto-pts/auto-pts that referenced this pull request Dec 4, 2024
Aligning the bot for mesh blob client tests with zephyr
tester.

zephyrproject-rtos/zephyr#81531

Signed-off-by: alperen sener <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Bluetooth Mesh area: Bluetooth Qualification Bluetooth Qualification -related issues and pull requests area: Bluetooth
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants