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: Recalculate regulator internal sum #11713

Merged
merged 3 commits into from
Aug 21, 2023

Conversation

PavelVPV
Copy link
Contributor

@PavelVPV PavelVPV commented Jul 6, 2023

Recalculate internal sum of the regulator to mitigate rapid change of
the regulator output when lightness value is non zero.
Add ramp up time to the pi regulator to artificially delay the
regulator.

This PR additionally changes light controller unit test to use system
time instead of mocking kernel functions to test the regulator with
the fsm.

Commits are split to make easier to see changes related to new behavior.

@PavelVPV PavelVPV requested a review from omkar3141 July 6, 2023 09:35
@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 Jul 6, 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.

@PavelVPV PavelVPV changed the title Bluetooth: Mesh: Add ramp up time for the regulator Bluetooth: Mesh: Recalculate regulator internal sum Jul 24, 2023
@PavelVPV PavelVPV force-pushed the recalc_reg_int_sum branch 2 times, most recently from 958bb79 to 90c5e6f Compare July 24, 2023 08:04
@PavelVPV PavelVPV marked this pull request as ready for review July 24, 2023 08:05
@PavelVPV PavelVPV requested a review from ludvigsj as a code owner July 24, 2023 08:05
@PavelVPV
Copy link
Contributor Author

@omkar3141, please review.

@NordicBuilder
Copy link
Contributor

NordicBuilder commented Jul 24, 2023

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

Copy link
Contributor

@mia-ko mia-ko left a comment

Choose a reason for hiding this comment

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

Just a few things that caught my attention.

subsys/bluetooth/mesh/light_ctrl_reg_spec.c Outdated Show resolved Hide resolved
subsys/bluetooth/mesh/light_ctrl_reg_spec.c Outdated Show resolved Hide resolved
Copy link
Contributor

@ludvigsj ludvigsj left a comment

Choose a reason for hiding this comment

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

Only a tiny style comment, but not gonna hold approval because of it.

Use system time instead of mocking k_* functions.

Signed-off-by: Pavel Vasilyev <pavel.vasilyev@nordicsemi.no>
After switching the controller tests to system time, the regulator test
that checks regulator's wrap around behavior started taking too much
time.

Wisely hack the internal sum of the regulator to avoid long execution
time.

Signed-off-by: Pavel Vasilyev <pavel.vasilyev@nordicsemi.no>
Recalculate internal sum of the regulator to mitigate rapid change of
the regulator output when lightness value is non zero.
Add ramp up time to the pi regulator to artificially delay the
regulator.

Signed-off-by: Pavel Vasilyev <pavel.vasilyev@nordicsemi.no>
@rlubos rlubos merged commit 80f5f5f into nrfconnect:main Aug 21, 2023
12 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. changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants