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

Refactor ble advertising to capsule & kernel hil #810

Merged
merged 2 commits into from Mar 18, 2018

Conversation

cpluss
Copy link
Contributor

@cpluss cpluss commented Mar 13, 2018

Pull Request Overview

Refactor of the nrf5x BLE advertising driver to capsules, and the BLE advertising hil into the kernel hil.

No logic changed, just refactored into different cargo containers.

I think this is the correct locations, but might be wrong :)

Formatting

  • Ran make formatall.

cpluss and others added 2 commits March 13, 2018 19:55
Refactor of the nrf5x BLE advertising driver to capsules,
and the BLE advertising hil into the kernel hil.
@bradjc bradjc added the P-Upkeep This a relatively minor change, or one that is limited in scope, and requires less scrutiny. label Mar 14, 2018
@bradjc
Copy link
Contributor

bradjc commented Mar 14, 2018

Just a note, we setup the git pr template because it helps with getting things in order before submitting a pull request. In this case updates to the documentation are required because we have a list of capsules in the README to help users find what Tock supports (looking through a folder of 40-odd, sometimes cryptic file names is a little daunting), and adding a new capsule requires updating the list.

I pushed a commit that adds the link, but in the future it would be helpful if you left those checkboxes there.

@cpluss
Copy link
Contributor Author

cpluss commented Mar 14, 2018

@bradjc thanks! I'll think about this in the future :) This PR was a bit rushed on my part

Copy link
Member

@niklasad1 niklasad1 left a comment

Choose a reason for hiding this comment

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

LGTM!

But I leave the last word to @alevy because he had some concerns putting the ẀIP HAL in the capsules previously that's why is it in ǹrf5x currently

@alevy
Copy link
Member

alevy commented Mar 18, 2018

@niklasad1 I think the concerns I used to have about that are less important now that we have more formalized ways of listing which parts of the repo is stable and which is in flux. Plus, now that we have another BLE chip coming along, any issues with the interface not being generic enough should pop up quickly.

@alevy
Copy link
Member

alevy commented Mar 18, 2018

We'll almost certainly want to rename the HIL to ble_radio or something more general than just advertising, but moving to capsules as-is is a good first step as well.

@alevy alevy merged commit 66f676d into tock:master Mar 18, 2018
@cpluss cpluss deleted the refactor_ble_adv branch March 19, 2018 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-Upkeep This a relatively minor change, or one that is limited in scope, and requires less scrutiny.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants