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

samples: mesh: nrf52: improved coding style, way to save default & target values on flash #21014

Merged
merged 11 commits into from
Dec 11, 2019
Merged

samples: mesh: nrf52: improved coding style, way to save default & target values on flash #21014

merged 11 commits into from
Dec 11, 2019

Conversation

vikrant8052
Copy link
Contributor

  1. Removed redundant code.
  2. Implement thing based on single struct light_ctl_state pointer
    in entire Application.
  3. Rename attribute in enum state_binding : CTL to CTL_LIGHT
  4. Separately saved default & last target values of lightness,
    temperature & delta_uv on flash (using settings layer).

Signed-off-by: Vikrant More [email protected]

@vikrant8052 vikrant8052 requested a review from nashif as a code owner November 26, 2019 20:26
@zephyrbot zephyrbot added the area: Samples Samples label Nov 26, 2019
@zephyrbot
Copy link
Collaborator

zephyrbot commented Nov 26, 2019 •
edited
Loading

All checks are passing now.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

@vikrant8052
Copy link
Contributor Author

@cvinayak @carlescufi Are you able to connect to any Bluetooth Zephyr App over GATT (in case of any nRF52 board running latest master branch) ?

@jhedberg jhedberg added this to the v2.2.0 milestone Nov 28, 2019
@cvinayak
Copy link
Contributor

cvinayak commented Nov 28, 2019 via email

@vikrant8052
Copy link
Contributor Author

@cvinayak Let me test it again. Thank you for your reply.

@vikrant8052
Copy link
Contributor Author

@cvinayak
Just now I synced with master branch & now everything is working perfectly fine.

@cvinayak
Copy link
Contributor

@cvinayak
Just now I synced with master branch & now everything is working perfectly fine.

Thanks for testing. Please log a github issue and assign to me if you face issues in future related to controller. When assigned, its more visible to me on github.

@vikrant8052
Copy link
Contributor Author

Please don't merge. Small changes have to be done by adding new commit

@jhedberg
Copy link
Member

Please don't merge. Small changes have to be done by adding new commit

@Vikrant8051 I couldn't merge even if I wanted to since the project is in the stabilisation phase for Zephyr 2.2, i.e. only PRs that fix important bugs are allowed.

@vikrant8052
Copy link
Contributor Author

vikrant8052 commented Nov 30, 2019 •
edited
Loading

I've added new commits (#10 & #11).

@joerchan
Copy link
Contributor

joerchan commented Dec 6, 2019

@Vikrant8051 @jhedberg @trond-snekvik @carlescufi
This is looking more like an application and less like a sample. If it is an application I don't think it makes sense to keep this under samples here. If this is vikrants mesh application it would make more sense to have this in an independent repository probably, vikrant could then do as he pleases with his application without needed approval from all of us.

@vikrant8052
Copy link
Contributor Author

@joerchan It is due to rebase issue, system automatically send review request to many reviewers.
Sorry for the inconvenience.

@jhedberg
Copy link
Member

jhedberg commented Dec 6, 2019

@joerchan It is due to rebase issue, system automatically send review request to many reviewers.
Sorry for the inconvenience.

I think that's a bit beside the point @joerchan was trying to make. I've also had the same thought many times. Samples are supposed to be fairly small and static, i.e. they shouldn't need much updating. This sample has had numerous 1000+ line PRs since it was created, and the sample itself is rather huge. So I do think it's worth considering if it'd make sense to host it in an independent repository.

@vikrant8052
Copy link
Contributor Author

After merging this PR, App will be in most robust state.
Plz note that non of the other Mesh App is following PTS norm &
giving example of state transition, state binding.

@jhedberg
Copy link
Member

jhedberg commented Dec 9, 2019

@Vikrant8051 a PR was recently merged that did the following path rename: samples/boards/{nrf52 => nrf}. This means that you will have to rebase this PR before it can be merged.

@joerchan
Copy link
Contributor

joerchan commented Dec 9, 2019

After merging this PR, App will be in most robust state.
Plz note that non of the other Mesh App is following PTS norm &
giving example of state transition, state binding.

Alright. And then it should remain? Otherwise I think we should consider extracting it out.
Just my suggestion, I guess you MESH guys know the answer to this.

@vikrant8052
samples: mesh: removed redundant code, blank lines & comments
d624090 
Removed redundant code, unnecessary blank lines & comments.

Signed-off-by: Vikrant More <[email protected]>
@vikrant8052
samples: mesh: nrf52: implementation based on single pointer
c79080e 
Implement thing based on single struct light_ctl_state pointer
in entire Application.

Signed-off-by: Vikrant More <[email protected]>
@vikrant8052
samples: mesh: nrf52: rename enum attributes
cfb36bd 
Rename attributes in enum state_binding.

Signed-off-by: Vikrant More <[email protected]>
@vikrant8052
samples: mesh: nrf52: separately save default & last target values
b4b4ed4 
Separately saved default & last target values of lightness,
temperature & delta_uv on flash (using settings layer).

Signed-off-by: Vikrant More <[email protected]>
@vikrant8052
samples: mesh: nrf52: used constrain functions whereever possible
16c789c 
Added support of constrain_temperature() function.
Used constrain_lightness() & constrain_temperature()
whereever possible.

Signed-off-by: Vikrant More <[email protected]>
@vikrant8052
samples: mesh: nrf52: added separate update_vnd_led_gpio()
8f0e1b6 
Added sepate function update_vnd_led_gpio().

Signed-off-by: Vikrant More <[email protected]>
@vikrant8052
samples: mesh: nrf52: corrected sequence of get & publish messages
0f359d2 
Corrected sequence of execution of get & publish messages.

Signed-off-by: Vikrant More <[email protected]>
@vikrant8052
samples: mesh: nrf52: removed redundant global variable
9456a63 
Removed global variable 'default_tt' & code depend on it
which is redundant as per latest implementation.

Signed-off-by: Vikrant More <[email protected]>
@vikrant8052
samples: mesh: nrf52: added some more macros support
f8e8b87 
Added some more macros support.

Signed-off-by: Vikrant More <[email protected]>
@vikrant8052
samples: mesh: nrf52: minor improvement in MOVE msg handlers
8453ab0 
Changes in MOVE message handler are  as per Mesh Model
Specification which says:

"Upon receiving a Generic Move Set message, the Generic Level
Server shall respond with a Generic Level Status message.
The target Generic Level state is the upper limit of the
Generic Level state when the transition speed is positive,
or the lower limit of the Generic Level state
when the transition speed is negative."

Signed-off-by: Vikrant More <[email protected]>
@vikrant8052
samples: mesh: nrf52: added some preprocessor directive
d89f054 
Added some preprocessor directive so that code get compile
for some more nRF52 boards which has only one LED & one button.

Signed-off-by: Vikrant More <[email protected]>
@vikrant8052
Copy link
Contributor Author

vikrant8052 commented Dec 9, 2019 •
edited
Loading

@Vikrant8051 a PR was recently merged that did the following path rename: samples/boards/{nrf52 => nrf}. This means that you will have to rebase this PR before it can be merged.

@jhedberg Done !!

@jhedberg jhedberg merged commit 5af5407 into zephyrproject-rtos:master Dec 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trond-snekvik trond-snekvik trond-snekvik approved these changes

@jhedberg jhedberg jhedberg approved these changes

@nashif nashif Awaiting requested review from nashif

@joerchan joerchan Awaiting requested review from joerchan

@Vudentz Vudentz Awaiting requested review from Vudentz

Assignees
No one assigned
Labels
area: Bluetooth Mesh area: Samples Samples
Projects
None yet
Milestone
v2.2.0
Development

Successfully merging this pull request may close these issues.
6 participants
@vikrant8052 @zephyrbot @cvinayak @jhedberg @joerchan @trond-snekvik