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

Support for Model Publication in Light CTL Temperature Server #19496

Merged

Conversation

IlijaVorontsov
Copy link
Contributor

Specification Requirements

| From the Mesh Model Specification v1.0.1, Section 6.4.4.1:

This model shall support model publication, as defined in Section 4.2.2 of the Mesh Profile specification.

The Mesh Profile Specification v1.0.1, Section 4.2.2 defines model publication as a composite state that controls message publishing parameters, including:

Publish Address
Publish Period
Retransmission Settings

Implementation Approach

The implementation adds publication support following the same pattern established in the light_ctl_srv, ensuring consistency with existing model implementations while fulfilling the specification's requirements. This approach maintains architectural coherence within the Bluetooth mesh stack.

@IlijaVorontsov IlijaVorontsov requested a review from a team as a code owner December 13, 2024 09:24
@CLAassistant
Copy link

CLAassistant commented Dec 13, 2024

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. ble mesh Label for ble mesh PRbot. Add this if PR is related to ble mesh and you need to get review. labels Dec 13, 2024
@NordicBuilder
Copy link
Contributor

Thank you for your contribution!
It seems you are not a member of the nrfconnect GitHub organization. External contributions are handled as follows:
Large contributions, affecting multiple subsystems for example, may be rejected if they are complex, may introduce regressions due to lack of test coverage, or if they are not consistent with the architecture of nRF Connect SDK.
PRs will be run in our continuous integration (CI) test system.
If CI passes, PRs will be tagged for review and merged on successful completion of review. You may be asked to make some modifications to your contribution during review.
If CI fails, PRs may be rejected or may be tagged for review and rework.
PRs that become outdated due to other changes in the repository may be rejected or rework requested.
External contributions will be prioritized for review based on the relevance to current development efforts in nRF Connect SDK. Bug fix PRs will be prioritized.
You may raise issues or ask for help from our Technical Support team by visiting https://devzone.nordicsemi.com/.

Note: This comment is automatically posted and updated by the Contribs GitHub Action.

@NordicBuilder NordicBuilder added the external External contribution label Dec 13, 2024
Copy link
Contributor

@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 good. However, commit message needs to follow the format as used by other commits in these files, should be bit more descriptive, and should be signed-off.

@IlijaVorontsov IlijaVorontsov force-pushed the Light-CTL-Server-Periodic-Publishing branch from 514bae5 to 4624730 Compare December 18, 2024 11:10
Copy link
Contributor

@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.

@omkar3141's concerns have not been fixed either.

I triggered CI, please take a look at the compliance bugs. They have to be fixed.

Please rebase at the latest main to avoid another issues.

subsys/bluetooth/mesh/light_temp_srv.c Outdated Show resolved Hide resolved
@omkar3141
Copy link
Contributor

omkar3141 commented Jan 14, 2025

Hello, any updates on this PR? I agree about temp_get() comment from Alex, though I wouldn't block it for that. This PR is relevant and it will be nice to fix review comments and then we can get this in. Thanks.

@IlijaVorontsov
Copy link
Contributor Author

Hello, any updates on this PR? It is relevant and it will be nice to fix review comments and then we can get this in. Thanks.

Hi, just set down to propperly write the commit messages with attribution

@IlijaVorontsov IlijaVorontsov force-pushed the Light-CTL-Server-Periodic-Publishing branch 3 times, most recently from 9259171 to fdadecb Compare January 14, 2025 12:22
@IlijaVorontsov
Copy link
Contributor Author

Sorry about the delay, struggled a bit with amending the commit messages correctly whilst conforming to the contribution guidelines. I messed up with the built in attribution of Alex, but added him as co-author to the commit. Hope that is fine.

@IlijaVorontsov IlijaVorontsov force-pushed the Light-CTL-Server-Periodic-Publishing branch 3 times, most recently from 14acca0 to 95cf766 Compare January 17, 2025 11:20
Implemented model publication as specified in Mesh Model Specification
v1.0.1, Section 6.4.4.1. Implemented similar to other publication
services in Bt Mesh. Tested in company own testing environment.

Co-authored-by: Aleksandr Khromykh <aleksandr.khromykh@nordicsemi.no>
Signed-off-by: Ilija Vorontsov <ilija.vorontsov@loytec.com>
@IlijaVorontsov IlijaVorontsov force-pushed the Light-CTL-Server-Periodic-Publishing branch from 95cf766 to 83fe35d Compare January 17, 2025 12:18
@alxelax alxelax added CI-Requested Approves single commit for CI tests on Internal HW and removed changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Jan 17, 2025
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Jan 20, 2025

CI Information

To view the history of this post, clich the 'edited' button above
Build number: 1

Inputs:

Sources:

sdk-nrf: PR head: 83fe35d8e1e223a1d05818b160df68158993652c

more details

sdk-nrf:

PR head: 83fe35d8e1e223a1d05818b160df68158993652c
merge base: e1b6e2afc766aa23cf1bebabdec52d75fd4ae648
target head (main): 5e125259f131cb4b5b7c1c72588df0fd0ae7f0c1
Diff

Github labels

Enabled Name Description
ci-disabled Disable the ci execution
ci-all-test Run all of ci, no test spec filtering will be done
ci-force-downstream Force execution of downstream even if twister fails
ci-run-twister Force run twister
ci-run-zephyr-twister Force run zephyr twister
List of changed files detected by CI (2)
include
│  ├── bluetooth
│  │  ├── mesh
│  │  │  │ light_temp_srv.h
subsys
│  ├── bluetooth
│  │  ├── mesh
│  │  │  │ light_temp_srv.c

Outputs:

Toolchain

Version: 342151af73
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:342151af73_912848a074

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ✅ Build twister
    • sdk-nrf test count: 820
  • ✅ Integration tests
    • ✅ desktop52_verification
    • ✅ test-fw-nrfconnect-ble_mesh
Disabled integration tests
    • doc-internal
    • test_ble_nrf_config
    • test-fw-nrfconnect-apps
    • test-fw-nrfconnect-ble_samples
    • test-fw-nrfconnect-boot
    • test-fw-nrfconnect-chip
    • test-fw-nrfconnect-fem
    • test-fw-nrfconnect-nfc
    • test-fw-nrfconnect-nrf-iot_libmodem-nrf
    • test-fw-nrfconnect-nrf-iot_lwm2m
    • test-fw-nrfconnect-nrf-iot_mosh
    • test-fw-nrfconnect-nrf-iot_positioning
    • test-fw-nrfconnect-nrf-iot_samples
    • test-fw-nrfconnect-nrf-iot_serial_lte_modem
    • test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
    • test-fw-nrfconnect-nrf_crypto
    • test-fw-nrfconnect-ps
    • test-fw-nrfconnect-rpc
    • test-fw-nrfconnect-rs
    • test-fw-nrfconnect-tfm
    • test-fw-nrfconnect-thread
    • test-fw-nrfconnect-zigbee
    • test-low-level
    • test-sdk-audio
    • test-sdk-dfu
    • test-sdk-find-my
    • test-sdk-mcuboot
    • test-sdk-pmic-samples
    • test-sdk-sidewalk
    • test-sdk-wifi
    • test-secdom-samples-public

Note: This message is automatically posted and updated by the CI

@nordicjm nordicjm merged commit a2f01a9 into nrfconnect:main Jan 20, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ble mesh Label for ble mesh PRbot. Add this if PR is related to ble mesh and you need to get review. CI-Requested Approves single commit for CI tests on Internal HW external External contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants