espressif / esp-homekit-sdk

550 stars 99 forks source link

Added a KConfig option to set NVS partition names for homekit use #32

Closed ryankurte closed 3 years ago

ryankurte commented 3 years ago

Hey thanks for working on this!

Just playing with homekit things and discovered a bit of a hiccup where one already has an NVS partition (or three) and the homekit API expects to be able to read, write, and erase nvs and nvs_factory partitions...

This PR adds KConfig options to set the partition names, with defaults to match the original approach, so it can be used alongside any existing nvs partitions. IMO it'd be useful to dependency inject the k/v store so it could be integrated with other approaches to configuration, but for now this solves the interoperability problem.

CLAassistant commented 3 years ago

CLA assistant check
All committers have signed the CLA.

ryankurte commented 3 years ago

thanks for the review, that should be all of em ^_^

shahpiyushv commented 3 years ago

@ryankurte , this looks good, but it would help if you could squash the 2 commits together (instead of me doing it while merging internally), so that github can map the commit to this PR directly.

shahpiyushv commented 3 years ago

@ryankurte , this has been merged in this commit: https://github.com/espressif/esp-homekit-sdk/commit/86bda138ef77941bd651eda49878cb5b8e5aff2b

Strangely enough, the commit id had changed and so it did not reflect here directly.

ryankurte commented 3 years ago

how strange... thanks for merging it in!