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

Jump to bottom

samples: mesh: enable SMP service for DFU [v1.14.0 milestone] #13135

Merged
merged 1 commit into from Apr 17, 2019
Merged

samples: mesh: enable SMP service for DFU [v1.14.0 milestone] #13135

merged 1 commit into from Apr 17, 2019

Conversation

vikrant8052
Copy link
Contributor

This commit enable Bluetooth SMP Service which will help
us to do Device Firmware Upgrade over thr air. By default
it is not enable.

Reference: $zephyr/samples/subsys/mgmt/mcumgr/smp_svr

Signed-off-by: Vikrant More vikrant8051@gmail.com

@vikrant8052
Copy link
Contributor Author

vikrant8052 commented Feb 7, 2019 •
edited

@jhedberg
As per your suggestion, I have created this separate PR because it is going to introduce new feature in #onoff_level_lighting_vnd_App.

@carlescufi @nvlsianpu @ccollins476ad @jhedberg @SebastianBoe @mbolivar
Kindly review this PR thoroughly & help me to improve the implementation.

For reference I have used: $zephyr/samples/subsys/mgmt/mcumgr/smp_svr.

As per my opinion, this should be a part of 1.14 :)

@vikrant8052
Copy link
Contributor Author

@mbolivar
Thanks for initiating the review process.

As of now, implementing things as per your suggestions is beyond my scope.

So please bare me !!

I don't wanna make this App by default compatible for #mcuboot
User should simply build & run it without prior flashing of bootloader.

@codecov-io
Copy link

codecov-io commented Feb 7, 2019 •
edited

Codecov Report

Merging #13135 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #13135   +/-   ##
=======================================
  Coverage   48.76%   48.76%           
=======================================
  Files         319      319           
  Lines       46892    46892           
  Branches    10840    10840           
=======================================
  Hits        22867    22867           
  Misses      19391    19391           
  Partials     4634     4634

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7fce83f...28ffbed. Read the comment docs.

@mbolivar
Copy link
Contributor

mbolivar commented Feb 7, 2019

@mbolivar
Thanks for initiating the review process.

No problem.

This is not my area in Zephyr but I've tried to leave you a few more hints.

nvlsianpu
nvlsianpu reviewed Feb 8, 2019
View reviewed changes
samples/boards/nrf52/mesh/onoff_level_lighting_vnd_app/CMakeLists.txt Outdated
@@ -20,3 +23,5 @@ zephyr_include_directories(
src/
src/mesh
)

zephyr_sources_ifdef(CONFIG_BOOTLOADER_MCUBOOT src/smp_svr.c)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not depends on CONFIG_MCUMGR istead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done !!

@SebastianBoe SebastianBoe removed their request for review February 11, 2019 08:36
@vikrant8052 vikrant8052 changed the title samples: mesh: enable SMP service for DFU samples: mesh: enable SMP service for DFU [v1.14.0 milestone] Feb 11, 2019
mbolivar
mbolivar previously requested changes Feb 12, 2019
View reviewed changes
Copy link
Contributor

@mbolivar mbolivar left a comment

Choose a reason for hiding this comment

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

Structurally I just see one more issue with this PR. After that is resolved I think it is up to the application maintainer.

samples/boards/nrf52/mesh/onoff_level_lighting_vnd_app/CMakeLists.txt Outdated Show resolved Hide resolved
@vikrant8052
samples: mesh: enable SMP service for DFU
28ffbed 
This commit enable Bluetooth SMP Service which will help
us to do Device Firmware Upgrade over thr air. By default
it is not enable.

Reference: $zephyr/samples/subsys/mgmt/mcumgr/smp_svr

Signed-off-by: Vikrant More <vikrant8051@gmail.com>
@vikrant8052
Copy link
Contributor Author

@mbolivar Done !! And thanks for more appropriate suggestion.

@mbolivar
Copy link
Contributor

@jhedberg @nvlsianpu I see no obvious issues with this pull request, so I've dismissed -1 and will leave it up to you from here.

@nvlsianpu
Copy link
Collaborator

I see no obstacles for this PR merge.
@jhedberg - this is mesh example, so pleas look at it.

@jhedberg
Copy link
Member

@jhedberg - this is mesh example, so pleas look at it.

@nvlsianpu haven't forgotten about this, but since it's not a bug fix I think it may need to wait until 1.14 is released.

@nvlsianpu
Copy link
Collaborator

^^ yes.

@jhedberg jhedberg merged commit 47b7c79 into zephyrproject-rtos:master Apr 17, 2019
@rlubos rlubos mentioned this pull request Apr 23, 2019
Merged
@zakariae87
Copy link

zakariae87 commented Feb 14, 2020 •
edited

This sample (/zephyrproject/zephyr/samples/boards/nrf/mesh/onoff_level_lighting_vnd_app) doesnt build corrcectly under zephyr 2.1.99

@jhedberg
Copy link
Member

This sample (/zephyrproject/zephyr/samples/boards/nrf/mesh/onoff_level_lighting_vnd_app) doesnt build corrcectly under zephyr 2.1.99

@zakariae87 I just tried building it with latest master and had no problems (for board nrf52840_pca10056)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nvlsianpu nvlsianpu nvlsianpu left review comments

@jhedberg jhedberg jhedberg approved these changes

@mbolivar mbolivar mbolivar left review comments

@carlescufi carlescufi Awaiting requested review from carlescufi

Assignees
No one assigned
Labels
area: Bluetooth
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@vikrant8052 @codecov-io @mbolivar @nvlsianpu @jhedberg @zakariae87 @aescolar