ndokter / dsmr_parser

Library to parse Dutch Smart Meter Requirements (DSMR) telegrams.
MIT License
112 stars 63 forks source link

Refactor of telegram_specifications.py and added E.ON Hungary specification #136

Closed balazs92117 closed 1 year ago

balazs92117 commented 1 year ago

Follow up of #134

So I did the followings:

//TODO

  1. If this refactor is ok with you @ndokter, I will change the other specifications too.
  2. I would suggest to change obis_references.py from names to obis codes. So e.g.: OBIS_028 instead of P1_MESSAGE_HEADER. With this modificaitons the names wouldn't be so confusing, as a lot of specification uses the same obis code for different values (under different name). This is a very big BC, which would break e.g.: Home Assistant integration, so maybe we shouldn't do this in this PR.
  3. I would merge multiple name mappings which are the same. e.g.: Q3D_EQUIPMENT_SERIALNUMBER and EON_HU_EQUIPMENT_SERIAL_NUMBER to EQUIPMENT_SERIAL_NUMBER or the lots of *EQUIPMENT_IDENTIFIERs . Just in the name mappings in EN, as the obis references are heavily used in e.g.: HA integration.
ndokter commented 1 year ago

It looks good, thanks for your hard work. Made 1 comment that we should maintain backwards compatibility for now

balazs92117 commented 1 year ago

Also I would skip both 2. and 3. of the TODO list, as I have no idea what would this affect (I checked the HA integration and it is a bit confusing). So I add the new names/references with EON_HU prefix, and use the already existing ones where the name is exactly the same. See the latest commit.

ndokter commented 1 year ago

Ok maybe we can do the following, let me know what you think. Ill try to summarize all thoughts in this comment, so i wont clutter the PR too much:

Is this possible or am i missing something? And would you want to make all the needed changes?

Nigel

balazs92117 commented 1 year ago

Ah alright, I finally understand what you meant before. Yeah sure I can do this, and I aggree with the suggestions regarding the renames of OBIS references. Let me do it and create a new MR.