Conversation
added 02-8E
nao-pon
left a comment
There was a problem hiding this comment.
Thanks for the contribution — adding support for EOJ 0x028E looks useful and aligns with the ECHONET Lite Appendix update.
Before merging, could you please revert the changes in pychonet/version.py?
Version management is handled separately during the release process, and the modification in this PR appears unrelated to the feature being added here.
Also, please see the comment below regarding the EPC_FUNCTIONS definition so it matches the existing pattern used in other device classes.
Once those points are addressed, I will review the changes again.
The final merge will likely be done using squash, so the exact commit structure is not critical.
Thanks again for the contribution!
| EPC_FUNCTIONS = { | ||
| 0x80: _int, # Operation status | ||
| 0x98: _yyyy_mm_dd, # Current date setting | ||
| 0xD4: _028ED4, # Unit for cumulative amount of electric energy |
There was a problem hiding this comment.
For EPC 0xD4 (unit for cumulative electric energy), it would be better to follow the existing EPC_FUNCTIONS pattern used in other energy-related classes.
In pychonet, EPCs that typically defined using the [decoder, multiplier_map, default_value] structure.
This keeps the definition consistent with other device classes and allows integrations to correctly interpret and apply the unit when processing the values.
Something like the following would be consistent with the rest of the implementation:
0xD4: [
_int,
{
0x00: 1,
0x01: 0.1,
0x02: 0.01,
0x03: 0.001,
0x04: 0.0001,
0x0A: 10,
0x0B: 100,
0x0C: 1000,
0x0D: 10000,
},
None,
]This is similar to how other device classes handle values.
| _yyyy_mm_dd, | ||
| ) | ||
|
|
||
| def _028ED4(edt): |
There was a problem hiding this comment.
This definition can be removed by changing.
nao-pon
left a comment
There was a problem hiding this comment.
One more note: EOJ 0x028E appears to have additional properties defined in the ECHONET Lite Appendix that are not implemented in this PR yet.
I do not think that should block this PR, especially since other device classes in pychonet are also implemented incrementally. However, it may be worth considering those properties in a follow-up PR if needed.
|
Proceeding to make a better PR. |
added device "distributed generator’s electric energy meter class" (028E) according to echonet lite appendix release R rev3. This device must be presented in pychonet or HA can't add Kyocera all-in-one solar system.