Support DSMR data read via RFXtrx with integrated P1 reader about dsmr_parser HOT 11 CLOSED

Hey, yes i have to manually make a new release. Will do it soon. Thanks for the contribution!

from dsmr_parser.

Implemented a proof-of-concept, which works on my RFXtrx setup (with an additional single-line hack on the protocol level: changing DSMRProtocol into RFXtrxDSMRProtocol).
I can add the DSMR integration in Home Assistant without any problems after loading this change in my Home Assistant test environment, and the measured values are getting updated.

To finish the PR we would need to decide on what the external API should be:

• Add additional functions to the API (e.g. create_rfxtrx_dsmr_reader and create_rfxtrx_tcp_dsmr_reader)
• Add an additional parameter that specifies the whether or not to use the RFXtrx wrapper (could default to False)

from dsmr_parser.

Would it be possible to put the RFX specific part into a (new?) client module? It could inherit from the DSMRProtocol if needed i think. This way the RFX specific code is contained in it's own module and the default library is unaffected.

from dsmr_parser.

Yes, I think that could be possible.
Do you have any suggestions for the file structure and API you'd like to see?
E.g. something like rfxtrx_protocol.py containing create_rfxtrx_dsmr_reader, create_rfxtrx_tcp_dsmr_reader and create_rfxtrx_dsmr_protocol would make sense to me.
This would probably give a small bit of code duplication that could be avoided by using an additional parameter.
Maybe we could keep the external API the same and create some private functions with that extra parameter to keep the duplication minimal?

from dsmr_parser.

Those names sound good! What part would need to be duplicated?

from dsmr_parser.

In the current implementation only line 48 needs to be changed from DSMRProtocol into RFXtrxDSMRProtocol to make it work. Creating a private create_dsmr_protocol passing this as parameter would avoid duplication there.
The other two functions would need to call that with the appropriate parameter.

from dsmr_parser.

Maybe you can add a 'protocol' parameter?
def create_dsmr_protocol(dsmr_version, telegram_callback, loop=None, protocol=DSMRProtocol , **kwargs):

Which then is overrided only in the create_rfx.. function? Then the other methods won't work with the RFXtrxDSMRProtocol object which might be confusing?

from dsmr_parser.

I updated the (draft) Pull Request. Slightly differently than what you suggested. If you prefer, I can easily update it.

from dsmr_parser.

Nice! Same line of thought :)

from dsmr_parser.

I tried following your suggestion, but somehow Python will not let me use protocol=DSMRProtocol in the function declaration. I do not know enough Python to understand why.

Is there anything I should change/add before you would accept the PR?

from dsmr_parser.

Great to see the Pull Request getting merged. Thanks!
Do you plan to release this as a new version too?

from dsmr_parser.