Skip to content

BLE Host - Persist bonding material to sys/config#280

Merged
ccollins476ad merged 1 commit intoapache:masterfrom
ccollins476ad:store-config
Jun 28, 2017
Merged

BLE Host - Persist bonding material to sys/config#280
ccollins476ad merged 1 commit intoapache:masterfrom
ccollins476ad:store-config

Conversation

@ccollins476ad
Copy link
Contributor

This pull request adds a new package: ble/host/store/config. This package holds security material and CCCDs in RAM, and optionally saves them to sys/config. Because this package is a superset of store/ram, it deprecates that one.

This PR incorporates changes from other PRs:

Therefore, this PR should only be merged after those.

@ccollins476ad ccollins476ad force-pushed the store-config branch 2 times, most recently from 598a437 to 9af2789 Compare May 17, 2017 00:00
@mkiiskila
Copy link
Contributor

I could review this. Do you mind resolving the conflicts first, given that the other PRs have been merged? That would make the patch easier to read :)

@ccollins476ad
Copy link
Contributor Author

Thanks, Marko. I didn't realize so many conflicts had sprouted. They should be resolved now.

dst += idx * value_size;
src = dst + value_size;

move_count = *num_values - idx;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be multiplied by value_size?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good catch. I will fix that and see if I can add some unit tests for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It turns out there already were unit tests, but they were using the old deprecated store package (net/nimble/host/store/ram)!

In addition to the above change, I also had to reduce some setting defaults:

  • BLE_STORE_MAX_BONDS: 3 (was: 4)
  • BLE_STORE_MAX_CCCDS: 8 (was: 16)

The problem is that sys/config limits a value to <= 256 bytes. We should probably make this configurable, but even if we do, we don't have a good way to ensure these settings are in sync. The best we can do is raise an error with #if/#error.

@ccollins476ad ccollins476ad merged commit e416c09 into apache:master Jun 28, 2017
@ccollins476ad ccollins476ad deleted the store-config branch June 28, 2017 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants