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.

Sign up for GitHub

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

Jump to bottom

Bluetooth: Mesh: Develop/fixing light lc sm conditions #12040

Merged
merged 2 commits into from
Aug 23, 2023
Merged

Bluetooth: Mesh: Develop/fixing light lc sm conditions #12040

merged 2 commits into from
Aug 23, 2023

Conversation

Balaklaka
Copy link
Contributor

According to sections 6.2.3.5 of MshMDLd1.1r12, the value of the
Light LC Ambient LuxLevel state is set to 0 at power-up and when
the node is provisioned on the network.
According to sections 6.2.5.12 and 6.2.5.7 the Occupancy On should
not trigger any transition.

Signed-off-by: Ingar Kulbrandstad ingar.kulbrandstad@nordicsemi.no

@Balaklaka
Bluetooth: Mesh: Moved case statment as code was the same
850c7a7 
The code for the two case statements was the same, so merged
the case statement to use the same code.

Signed-off-by: Ingar Kulbrandstad <ingar.kulbrandstad@nordicsemi.no>
@github-actions github-actions bot added ble mesh Label for ble mesh PRbot. Add this if PR is related to ble mesh and you need to get review. changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Aug 15, 2023
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Aug 15, 2023 •
edited
Loading

Test specification

CI/Jenkins/NRF

  • Integration Platforms

CI/Jenkins/integration

Test Module File based changes Manually selected West overwrite
test-fw-nrfconnect-ble_mesh X

Detailed information of selected test modules

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

@Balaklaka Balaklaka removed the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Aug 15, 2023
@akredalen akredalen self-requested a review August 15, 2023 15:22
@Balaklaka Balaklaka changed the title Develop/fixing light lc sm conditions Bluetooth: Mesh: Develop/fixing light lc sm conditions Aug 16, 2023
PavelVPV
PavelVPV reviewed Aug 17, 2023
View reviewed changes
subsys/bluetooth/mesh/light_ctrl_srv.c Outdated Show resolved Hide resolved
subsys/bluetooth/mesh/light_ctrl_srv.c Outdated Show resolved Hide resolved
omkar3141
omkar3141 requested changes Aug 17, 2023
View reviewed changes
subsys/bluetooth/mesh/light_ctrl_srv.c Outdated Show resolved Hide resolved
subsys/bluetooth/mesh/light_ctrl_srv.c Outdated Show resolved Hide resolved
@Balaklaka Balaklaka force-pushed the develop/fixing-light-lc-sm-conditions branch from 1ca62bc to 51a94ae Compare August 18, 2023 13:22
@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Aug 18, 2023
@NordicBuilder
Copy link
Contributor

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publishing GitHub Action.

@Balaklaka Balaklaka force-pushed the develop/fixing-light-lc-sm-conditions branch from 51a94ae to 9472fb9 Compare August 21, 2023 08:35
omkar3141
omkar3141 approved these changes Aug 22, 2023
View reviewed changes
subsys/bluetooth/mesh/light_ctrl_srv.c
* should not be processed.
*/
(srv->state == LIGHT_CTRL_STATE_ON &&
atomic_test_bit(&srv->flags, FLAG_TRANSITION))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks fine now.

@Balaklaka Balaklaka force-pushed the develop/fixing-light-lc-sm-conditions branch from 9472fb9 to 90c9696 Compare August 22, 2023 08:35
ludvigsj
ludvigsj requested changes Aug 23, 2023
View reviewed changes
subsys/bluetooth/mesh/light_ctrl_srv.c Outdated
!atomic_test_bit(&srv->flags, FLAG_MANUAL))) {
if ((srv->state == LIGHT_CTRL_STATE_STANDBY &&
/* According to the Mesh Model Specification section
* 6.2.5.6 specifications STANDBY state, and the Auto
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* 6.2.5.6 specifications STANDBY state, and the Auto
* 6.2.5.6 about the STANDBY state, if the Auto

Copy link
Contributor

Choose a reason for hiding this comment

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

Same below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not agree in this change, because it is importent to reference to the specifications STANDBY state and not the STANDBY state in the code.

Copy link
Contributor

Choose a reason for hiding this comment

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

The sentence as it stands makes no sense. What is the "and" related to? The "and" makes no sense. What are you trying to say? "According to section 6.2.5.6: when you are in the specifications STANDBY state and the Auto Occupancy is off, the event should not be processed"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you have understood it correctly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Then:

Suggested change
* 6.2.5.6 specifications STANDBY state, and the Auto
* 6.2.5.6: When in the specifications STANDBY state, and the Auto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is becoming nitpicking, but I will add "When in the".

@Balaklaka
Bluetoth: Mesh: Fixing Light LC SM conditions
d598a55 
According to sections 6.2.3.5 of MshMDLd1.1r12, the value of the
Light LC Ambient LuxLevel state is set to 0 at power-up and when
the node is provisioned on the network.
According to sections 6.2.5.12 and 6.2.5.7 the Occupancy On should
not trigger any transition.

Signed-off-by: Ingar Kulbrandstad <ingar.kulbrandstad@nordicsemi.no>
@Balaklaka Balaklaka force-pushed the develop/fixing-light-lc-sm-conditions branch from 90c9696 to d598a55 Compare August 23, 2023 08:22
@rlubos rlubos merged commit 15ad0fe into nrfconnect:main Aug 23, 2023
12 checks passed
@Balaklaka Balaklaka deleted the develop/fixing-light-lc-sm-conditions branch August 23, 2023 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@omkar3141 omkar3141 omkar3141 approved these changes

@ludvigsj ludvigsj ludvigsj approved these changes

@PavelVPV PavelVPV PavelVPV approved these changes

@akredalen akredalen Awaiting requested review from akredalen

Assignees
No one assigned
Labels
ble mesh Label for ble mesh PRbot. Add this if PR is related to ble mesh and you need to get review. changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Balaklaka @NordicBuilder @PavelVPV @ludvigsj @omkar3141 @rlubos