zigpy / bellows

A Python 3 project to implement EZSP for EmberZNet devices
GNU General Public License v3.0
177 stars 87 forks source link

Prefer firmware config defaults and only grow them when necessary #550

Closed puddly closed 1 year ago

puddly commented 1 year ago

Default config is moved out of the protocol-specific definitions and into the main bellows application. With this change, two types of config are introduced:

  1. overrides: this config will always be set. For example, CONFIG_TC_REJOINS_USING_WELL_KNOWN_KEY_TIMEOUT_S will always be set to an exact value.
  2. minimum config: this config will only be changed if the current value is too low. Otherwise, the default value (set in firmware) is left alone.

Any of this behavior can be overridden by either overriding the config with an explicit value within your configuration, or by setting it to None to prevent bellows from changing it at all.

Fixes #543.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.04 :tada:

Comparison is base (a73588d) 99.71% compared to head (8fa5982) 99.75%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev #550 +/- ## ========================================== + Coverage 99.71% 99.75% +0.04% ========================================== Files 61 62 +1 Lines 4582 4583 +1 ========================================== + Hits 4569 4572 +3 + Misses 13 11 -2 ``` | [Impacted Files](https://codecov.io/gh/zigpy/bellows/pull/550?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zigpy) | Coverage Δ | | |---|---|---| | [bellows/ezsp/v8/config.py](https://codecov.io/gh/zigpy/bellows/pull/550?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zigpy#diff-YmVsbG93cy9lenNwL3Y4L2NvbmZpZy5weQ==) | `100.00% <ø> (ø)` | | | [bellows/multicast.py](https://codecov.io/gh/zigpy/bellows/pull/550?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zigpy#diff-YmVsbG93cy9tdWx0aWNhc3QucHk=) | `100.00% <ø> (ø)` | | | [bellows/zigbee/device.py](https://codecov.io/gh/zigpy/bellows/pull/550?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zigpy#diff-YmVsbG93cy96aWdiZWUvZGV2aWNlLnB5) | `100.00% <ø> (+3.92%)` | :arrow_up: | | [bellows/ezsp/config.py](https://codecov.io/gh/zigpy/bellows/pull/550?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zigpy#diff-YmVsbG93cy9lenNwL2NvbmZpZy5weQ==) | `100.00% <100.00%> (ø)` | | | [bellows/ezsp/protocol.py](https://codecov.io/gh/zigpy/bellows/pull/550?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zigpy#diff-YmVsbG93cy9lenNwL3Byb3RvY29sLnB5) | `97.36% <100.00%> (+0.12%)` | :arrow_up: | | [bellows/ezsp/v4/config.py](https://codecov.io/gh/zigpy/bellows/pull/550?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zigpy#diff-YmVsbG93cy9lenNwL3Y0L2NvbmZpZy5weQ==) | `100.00% <100.00%> (ø)` | | | [bellows/zigbee/application.py](https://codecov.io/gh/zigpy/bellows/pull/550?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zigpy#diff-YmVsbG93cy96aWdiZWUvYXBwbGljYXRpb24ucHk=) | `100.00% <100.00%> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zigpy). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zigpy)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

pipiche38 commented 1 year ago

Just checking, what does that means from a compatibility standpoint with the previous version ?

If I understood your PR, prior we had some restrictive values and now, by default we are following the default firmware values .

puddly commented 1 year ago

If I understood your PR, prior we had some restrictive values and now, by default we are following the default firmware values .

Yes. For example, if the firmware sets CONFIG_MULTICAST_TABLE_SIZE to 100, then it will be left at 100. If the firmware sets it to 2, it will be grown to 16. The old behavior is to always set it to 16.