Added Confirmed Restart #1445
Added Confirmed Restart #1445
Conversation
Changed procedures.nm_individual_address_write back to procedures.nm_invididual_address_write
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1445 +/- ##
==========================================
- Coverage 96.61% 96.44% -0.18%
==========================================
Files 151 151
Lines 9801 9862 +61
==========================================
+ Hits 9469 9511 +42
- Misses 332 351 +19
|
| RestartResponseError = [ | ||
| "No Error", | ||
| "Access denied", | ||
| "Unsupported Erase Code", | ||
| "Invalid Channel Number", | ||
| ] |
There was a problem hiding this comment.
This should be some kind of mapping (dict maybe) or enum, not a plain list.
Maybe move it to xknx/telegram/apci_const.py or xknx/management/const.py something like that.
|
|
||
| CODE = APCIService.CONFIRMED_RESTART | ||
|
|
||
| def __init__(self, erase_code: bytes, channel: bytes) -> None: |
There was a problem hiding this comment.
It would be handier to use int instead of bytes for channel and erase_code (or an enum for the latter if applicable).
Maybe set the default value of channel: int = 0 - for some of the erase codes other values are invalid so I guess it would be a proper default.
| CODE = APCIService.CONFIRMED_RESTART | ||
|
|
||
| def __init__(self, erase_code: bytes, channel: bytes) -> None: | ||
| """Initialize a new instance of FunctionPropertyCommand.""" |
There was a problem hiding this comment.
| """Initialize a new instance of FunctionPropertyCommand.""" | |
| """Initialize a new instance of ConfirmedRestart.""" |
|
|
||
| def __str__(self) -> str: | ||
| """Return object as readable string.""" | ||
| return "<Confirmed Restart />" |
There was a problem hiding this comment.
add the objects attributes so it is easier to debug
|
|
||
| CODE = APCIService.CONFIRMED_RESTART_RESPONSE | ||
|
|
||
| def __init__(self, error_code: bytes, time: bytes) -> None: |
There was a problem hiding this comment.
Consider using int or enum for error_code and int for time. Maybe rename time to time_sec or process_time (like in the specs).
| @classmethod | ||
| def from_knx(cls, raw: bytes) -> RestartResponse: | ||
| """Parse/deserialize from KNX/IP raw data.""" | ||
| error_code, time = struct.unpack("!BH", raw[2:]) |
There was a problem hiding this comment.
to en/decode time, you could use DPTTimePeriodSec.from_knx() and .to_knx() (but it is just a uint, so may not be necessary 🙃
| CONFIRMED_RESTART = b"\x01" | ||
| FACTORY_RESET = b"\x02" | ||
| RESET_IA = b"\x03" | ||
| RESET_AP = b"\x04" | ||
| RESET_PARAM = b"\x05" | ||
| RESET_LINKS = b"\x06" | ||
| FACTORY_RESET_WITHOUT_IA = b"\x07" |
There was a problem hiding this comment.
Consider using integers like in the specifications (eg. CONFIRMED_RESTART = 0x01)
Maybe move to xknx/telegram/apci_const.py or xknx/management/const.py something like that.
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions. |
Description
Added Confirmed Restart / Master Reset functionality
Testet with ETS6 Bus Monitor
Type of change
Checklist