Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added E.ON HUNGARY specification #134

Merged
merged 8 commits into from
Jul 26, 2023
Merged

Conversation

balazs92117
Copy link
Contributor

I have added new specification for E.ON service provider of Hungary.

Unfortunately the #80 issue is still exists, so I modified the clients/socket_.py
Until the service provider does not send a "normal" message instead of ? signs, we can't further test the issue.

@balazs92117
Copy link
Contributor Author

@ndokter Can you take a look at this pls?

@ndokter
Copy link
Owner

ndokter commented Jul 22, 2023

I would add a test like https://github.com/balazs92117/dsmr_parser/blob/eff0a5e1fc7065af902084e356fe6208fb563482/test/test_parse_v5.py maybe called test_parse_v5_hungary.py

Where we can validate that a complete eon/hungary telegram parsees into all expected values

@balazs92117
Copy link
Contributor Author

@ndokter
I added tests for the new parser. I called it test_parse_v5_eon_hungary.py as we have multiple service providers, not just "E.ON"

@ndokter ndokter merged commit 0752deb into ndokter:master Jul 26, 2023
0 of 5 checks passed
@ndokter
Copy link
Owner

ndokter commented Jul 26, 2023

Thanks for your work, i will make a new build soon

Edit: oh the tests broke, will look later

@balazs92117
Copy link
Contributor Author

Thank you! :)

ndokter added a commit that referenced this pull request Jul 26, 2023
ndokter added a commit that referenced this pull request Jul 26, 2023
@ndokter
Copy link
Owner

ndokter commented Jul 26, 2023

Hi im sorry i've had to revert the change for now. I noticed after merging that tests are breaking due to the belgium keys conflicting. After looking at it i think this is a more general issue that needs to be solved.

Maybe these need to be renamed to a more general key or something else needs to be thought of for dsmr_parser.obis_name_mapping.EN. Something like a translation per dsmr specification in here dsmr_parser.telegram_specifications

Or instead of that translation dict, use something more extensive than these key/values:

obis.P1_MESSAGE_HEADER: CosemParser(ValueParser(str)),

Maybe the translation value could be added there as well (using a new object). So in this case we could add 'P1_MESSAGE_HEADER' there. Could be an object or another nested dict:

{
    'obis_reference': P1_MESSAGE_HEADER,
    'value_parser': CosemParser(ValueParser(str)),
    'value_name': 'P1_MESSAGE_HEADER'
}

Do you know what i mean? What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants