ndokter / dsmr_parser

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

Add support for Iskra IE.5 (Belgium) #153

Open ljehu opened 2 months ago

ljehu commented 2 months ago

Modified regex in obis_references.py to recognize multiple belgian devices.

ndokter commented 2 months ago

Thanks for the PR! There are some tests which need to be fixed first

anthares commented 2 months ago

Hi @ljehu,

I just got the same smart meter installed but from another grid provider in Belgium (AIEG). I'd like to see this merged, but I don't know how I could help on this MR.

Feel free to let me know :)

Kind regards,

Nicolas

anthares commented 1 month ago

Hi @ndokter,

I made some test of this change locally to try to understand why tests are failing. I'm not a seasoned python dev (coming from Java world), but here is what I found.

Current change in version information regex makes parser fail to retrieve the right line in the telegram data. Instead of finding a single string, it finds a tuple of other strings, due to the addition on regex groups in the regex.

My feeling is, after some debug, is that just changing the regex won't work with the way the parser works right now. Making the parser compatible with those regex groups is way above my current skills. So I think the right way to go, and the safest, is to introduce another value for dsmr_version in protocol.py with corresponding specs in telegram_specifications.py and proper testing.

So, sorry to bother you on this, but I want your advice before going further. I'm ready to create another PR for that, I just want to make sure this is how it should be done :) I'm also open to suggestion for the dsmr_version value to introduce.

Best regards,

Nicolas

ndokter commented 1 month ago

Hi @anthares, sorry for the late response. I will try to get back to you

ndokter commented 3 weeks ago

@anthares you are right that the main problem is the tuple. Would something like this work? It fixes the tuple issue, but i'm not sure about the resulting values. The tests break in a different way: BELGIUM_VERSION_INFORMATION = r'^\d-\d:96\.(?:14\.0|1\.4).+?\r\n'