Skip to content

examples/nimble_*: use nimble_autoadv module #13506

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
Merged
merged 2 commits into from
Jul 21, 2021
Merged

examples/nimble_*: use nimble_autoadv module #13506

merged 2 commits into from
Jul 21, 2021

Conversation

HendrikVE
Copy link
Contributor

@HendrikVE HendrikVE commented Feb 28, 2020 •
edited
Loading

Contribution description

This PR modifies the example applications nimble_gatt and nimble_heart_rate_sensor such that they use the new automated advertising module, provided by PR #13425

Testing procedure

This PR is rebased on top of PR #13425, so you only have to build and test the applications mentioned above.

Issues/PRs references

Dependencies:

@HendrikVE HendrikVE added Area: BLE Area: Bluetooth Low Energy support Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: examples Area: Example Applications CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Feb 28, 2020
@HendrikVE HendrikVE changed the title Pr/autoadv transition examples/: use nimble_autoadv module Feb 28, 2020
@HendrikVE HendrikVE mentioned this pull request Feb 28, 2020
Merged
@HendrikVE HendrikVE force-pushed the pr/autoadv_transition branch from df28dcc to 5759d4b Compare February 28, 2020 11:59
@HendrikVE HendrikVE changed the title examples/: use nimble_autoadv module examples/nimble_*: use nimble_autoadv module Feb 28, 2020
@HendrikVE HendrikVE added the State: waiting for other PR State: The PR requires another PR to be merged first label Mar 5, 2020
@HendrikVE HendrikVE force-pushed the pr/autoadv_transition branch from 5759d4b to db323f6 Compare June 8, 2020 12:10
@HendrikVE HendrikVE force-pushed the pr/autoadv_transition branch 2 times, most recently from 83cf40f to c4db641 Compare November 5, 2020 11:24
@HendrikVE HendrikVE added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed State: waiting for other PR State: The PR requires another PR to be merged first CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 5, 2020
@stale
Copy link

stale bot commented Jun 2, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Jun 2, 2021
fjmolinas
fjmolinas approved these changes Jun 3, 2021
View reviewed changes
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

ACK, both applications still work as expected, @HendrikVE can you rebase so that the newer actions are triggered?

@stale stale bot removed the State: stale State: The issue / PR has no activity for >185 days label Jun 3, 2021
@fjmolinas
Copy link
Contributor

BTW this does come at the price of some code size:

  • master:
   text	   data	    bss	    dec	    hex	filename
  77220	    684	  10692	  88596	  15a14	/home/francisco/workspace/RIOT/examples/nimble_heart_rate_sensor/bin/nrf52840-mdk/nimble_heart_rate_sensor.elf
  • rebased pr:
   text	   data	    bss	    dec	    hex	filename
  77560	    684	  10756	  89000	  15ba8	/home/francisco/workspace/RIOT/examples/nimble_heart_rate_sensor/bin/nrf52840-mdk/nimble_heart_rate_sensor.elf

@HendrikVE HendrikVE force-pushed the pr/autoadv_transition branch from c4db641 to 2f10bef Compare June 3, 2021 12:43
@github-actions github-actions bot removed the Area: BLE Area: Bluetooth Low Energy support label Jun 3, 2021
@HendrikVE
Copy link
Contributor Author

@fjmolinas Thank you very much for testing my PR. I rebased to the newest master. Regarding the code size I think it's ok as the nrf52 has quite some flash storage and we are reducing code duplication :)

@fjmolinas
Copy link
Contributor

@fjmolinas Thank you very much for testing my PR. I rebased to the newest master. Regarding the code size I think it's ok as the nrf52 has quite some flash storage and we are reducing code duplication :)

Yep just wanted to state it.

@MrKevinWeiss MrKevinWeiss added this to the Release 2021.07 milestone Jun 21, 2021
@MrKevinWeiss MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021
@benpicco
Copy link
Contributor

All green here

@fjmolinas fjmolinas merged commit 7d829f1 into RIOT-OS:master Jul 21, 2021
@benpicco benpicco added this to the Release 2021.10 milestone Oct 20, 2021
@HendrikVE HendrikVE deleted the pr/autoadv_transition branch December 28, 2021 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fjmolinas fjmolinas fjmolinas approved these changes

@aabadie aabadie Awaiting requested review from aabadie

@jia200x jia200x Awaiting requested review from jia200x

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

Assignees
No one assigned
Labels
Area: examples Area: Example Applications CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Milestone
Release 2021.10
Development

Successfully merging this pull request may close these issues.
4 participants
@HendrikVE @fjmolinas @benpicco @MrKevinWeiss