Support for (German) EasyMeter Q3D using COM-1 Ethernet Gateway #92
Conversation
dsmr_parser/clients/protocol.py
Outdated
| @@ -99,7 +102,7 @@ def connection_made(self, transport): | |||
|
|
|||
| def data_received(self, data): | |||
| """Add incoming data to buffer.""" | |||
| data = data.decode('ascii') | |||
| data = data.decode('ascii', errors='ignore') | |||
There was a problem hiding this comment.
Is this the right location to solve this?
Is this safe for all other use cases as well?
I see for other use cases / readers we always still use the data.decode('ascii') variant. So this adds inconsistency as well.
Wouldn't it be better to have device specific input cleanups be done, before entering the generic parsing library?
There was a problem hiding this comment.
Yes, thought about it as well, but I saw a few identical or similar issues reported that this would fix and other forks solved it this way as well.
How would you imagine this would break things?
This is a quite simple fix.
Maybe additional checks further downstream could drop the telegram if needed.
There was a problem hiding this comment.
I don't know honestly.
But here we have one meter, that is adding a trailing \xFF.
The code is changed to ignore any decode error on the complete stream.
So suddenly it is allowing way more probably really erroneous (while un-decodeable) cases.
That does not sound like a good way of handling this.
What about the following solution; we add a new decode_ascii_telegram method, which:
- removes all trailing \xFF
data.rstrip(b'\xff') - invokes the
data.decode('ascii')
and invoke this in all places where currently data.decode('ascii') is used.
There was a problem hiding this comment.
See my other comment on the garbage added by the COM-1 boxes.
I would say there is sufficient downstream checks with _find_telegrams to ignore the garbage and with the CRC-checks to check the telegram integrity (if CRC is available). Futhermore the Regexs should also fail in cases where lines are incomplete (w/ or w/o CRC).
Unfortunatly \xff are not the last chars, but it's terminated by CR LF. Thus rstrip won't work.
I'm no expert here, but if I understand decode correctly it's just failing when it can't map a bytecode to target charset. It wouldn't catch any other "errors". So generally the "really erroneous" cases are quite limited, I would think. errors="replace" could be an option to "see" these cases.
Using "latin1" (8-bit) instead "ascii" (7-bit) works fine. Any concerns?
| @@ -181,3 +181,19 @@ | |||
| obis.INSTANTANEOUS_CURRENT_L3: CosemParser(ValueParser(Decimal)), | |||
| } | |||
| } | |||
|
|
|||
|
|
|||
| Q3D = { | |||
There was a problem hiding this comment.
Can you provide the specification for the protocol profile this meter is using? (A URL would be fine.)
(Maybe the name prefixes LUXEMBOURG_ is awkward, and we find out this is a more generally used item and can fix the names later on.)
There was a problem hiding this comment.
Here is some documentation I found:
EasyMeter Q3D Manual (german)
- type code on page 8
- D0-Specs on page 20
Implementation notes for a Q3D reader
- telegram analysis starting slide 15
The latter just calls "1.8.0" / "2.8.0" "positive" / "negative" active energy total.
So for my taste these
- ELECTRICITY_IMPORTED_TOTAL = r'\d-\d:1.8.0.+?\r\n'
- LUXEMBOURG_ELECTRICITY_USED_TARIFF_GLOBAL = r'\d-\d:1.8.0.+?\r\n' # Total imported energy register (P+)
- LUXEMBOURG_ELECTRICITY_DELIVERED_TARIFF_GLOBAL = r'\d-\d:2.8.0.+?\r\n' # Total exported energy register (P-)
- SWEDEN_ELECTRICITY_USED_TARIFF_GLOBAL = r'\d-\d:1.8.0.+?\r\n' # Total imported energy register (P+)
- SWEDEN_ELECTRICITY_DELIVERED_TARIFF_GLOBAL = r'\d-\d:2.8.0.+?\r\n' # Total exported energy register (P-)
could all be simplified to
- ELECTRICITY_IMPORTED_TOTAL = r'\d-\d:1.8.0.+?\r\n'
- ELECTRICITY_EXPORTED_TOTAL = r'\d-\d:2.8.0.+?\r\n'
But to not break things I would just add ELECTRICITY_EXPORTED_TOTAL and use them in the Q3D definition, instead of the LUXEMBOURG_ ones.
There was a problem hiding this comment.
This is the last thing we need to do, I think. The problem is that the parser can not distinguish between ELECTRICITY_IMPORTED_TOTAL, LUXEMBOURG_ELECTRICITY_USED_TARIFF_GLOBAL, SWEDEN_ELECTRICITY_USED_TARIFF_GLOBAL, as they are exactly the same regarding the matching pattern used.
Same for LUXEMBOURG_ELECTRICITY_DELIVERED_TARIFF_GLOBAL, SWEDEN_ELECTRICITY_DELIVERED_TARIFF_GLOBAL, ELECTRICITY_EXPORTED_TOTAL.
To unbreak things, we have to implement your simplified proposal. You want to give it a go?
There was a problem hiding this comment.
I do get your point, that the same code has different labels per different specifications. When I saw this for the first time, I tought it was a way to be more specific per each country using the codes slightly differently. But I could be wrong.
Since we excatly give the parser just one spec to run with (e.g. 5L), there is no harm for the parser. He only looks at that 5L spec. As long as ELECTRICITY_IMPORTED_TOTAL, LUXEMBOURG_ELECTRICITY_USED_TARIFF_GLOBAL, SWEDEN_ELECTRICITY_USED_TARIFF_GLOBAL don't show up in one spec, this should be fine. I have also not seen any issues reported on this.
So much to the conceptual part.
What remains is cosmetics to deduplicate the SWEDEN/LUXEMBOURG labels. I would still suggest that I do that in a separate pull request, to track issues on possible downstrem dependecies.
There was a problem hiding this comment.
If we are really bold, we may even go a bit further and see if we could remove the different specs entirely and unify them. This would remove the need to know which spec you need, remove the parameter/option and just deliver data based on a standard OBIS interpretation.
This would only work if the country/vendor specific codes are optional.
The parser could also "warn" if it discovers labels that are not part of the spec and not drop them, but forward them with "numeric" labels to be picked up downstream or enhancement of our spec here.
It might well be a breaking change, so I would suggest do that in a new branch. Also we would need to have folks at least from Sweden and Luxembourg to commit some time for testing, as I only have access to my two little boxes here.
Again, just to be be clear, these are just thoughts and goes well beyond this pull request.
Could you provide some more details on your setup here, I am curious how this works exactly.
|
|
Hi, I bought them a couple of years ago and can't find them on their website anymore either. They connect via optokopplers to the top of the EasyMeter and then allow you to listen on port 5000 tcp to the telegrams with an interval of two to three seconds between updates. Here is a picture:
|
|
Thanks. That is an interesting setup. You indicate that only the telegrams from the main meter are padded. Can we assume the padding is done by the meter itself? |
|
I have to guess so, as the gateways are the same. If you want, I could switch them and see. |
|
Up to you. I guess it interesting to know, but it doesn't help us further in solving the problem. |
|
Okay, I switched the meters connected to the COM-1 boxes. The garbage they add after the telegramm stays the same regardless of the Easymeter they connect to. After the telegramm end "! (CRC)\r\n", both add a "seconds counter" plus some static data, one with readable chars the other with the \xFF. As anything beyond the end "! (CRC)\r\n" is outside of scope of IEC 62056-21 Mode D, telegram_buffer._find_telegrams() already does the job of ignoring the garbage added by the COM-1 "transport layer". Below are two full datagrams received including the crc-less telegram and the COM-1-box-garbage: |
|
Accepting latin1 (8-bit char encoding) from the wire seems acceptable, we just have to make sure that the actual telegram is only defined for ascii (7-bit char encoding). |
Aeroid
changed the title
|
@lowdef you know more about this change than i do. Is the current change good and can it be merged? |
|
@ndokter i will have a look when i have some free time. Need to think about what is a good approach here. The charm of @Aeroid's solution is that it is very simple. The drawback might be that we have one particular specific ethernet gw specific fix in the generic code in one place only. Will come back on this soon. |
|
There is some parts I do not understand with the last changes in the code. This: try:
# we accepted 8-bit at transport level (e.g. tcp)
telegram_data = telegram.encode("latin1")
# we need to ensure 7-bit at telegram level (IEC 646 required in section 5.4 of IEC 62056-21)
telegram = telegram_data.decode("ascii")
parsed_telegram = self.telegram_parser.parse(telegram)I decided to give it a try with the telegrams you provided. Here is what i tried: #!/usr/bin/env python3
# -*- coding: utf-8 -*-
from dsmr_parser import telegram_specifications
from dsmr_parser.objects import Telegram
from dsmr_parser.parsers import TelegramParser
TELEGRAM_ESY5Q3DB1024_V304 = ( # Easymeter an Hauptstromzähler
'/ESY5Q3DB1024 V3.04\r\n'
'\r\n'
'1-0:0.0.0255(0272031312565)\r\n'
'1-0:1.8.0255(00052185.7825309kWh)\r\n'
'1-0:2.8.0255(00019949.3221493kWh)\r\n'
'1-0:21.7.0255(000747.85W)\r\n'
'1-0:41.7.0255(000737.28W)\r\n'
'1-0:61.7.0255(000639.73W)\r\n'
'1-0:1.7.0255(002124.86W)\r\n'
'1-0:96.5.5255(80)\r\n'
'0-0:96.1.255*255(1ESY1313002565)\r\n'
'!\r\n'
' 25803103\r\n'
'\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\r\n'
)
TELEGRAM_ESY5Q3DA1004_V304 = ( # Easymeter an Wärmepumpe
'/ESY5Q3DA1004 V3.04\r\n'
'\r\n'
'1-0:0.0.0255(1336001560)\r\n'
'1-0:1.8.0255(00032549.5061662kWh)\r\n'
'1-0:21.7.0255(000557.29W)\r\n'
'1-0:41.7.0255(000521.62W)\r\n'
'1-0:61.7.0255(000609.30W)\r\n'
'1-0:1.7.0255(001688.21W)\r\n'
'1-0:96.5.5255(80)\r\n'
'0-0:96.1.255*255(1ESY1336001560)\r\n'
'!\r\n'
' 25818685\r\n'
'DE0000000000000000000000000000003\r\n'
)
sample = TELEGRAM_ESY5Q3DB1024_V304
telegram_specification = telegram_specifications.Q3D
#telegram_specification['checksum_support'] = False
parser = TelegramParser(telegram_specification)
telegram = Telegram(sample, parser, telegram_specification)
print(telegram)We get following output: Probably the occurence of 255 in the lines causes this. So the pull request is not ready for merging yet. |
Character EncodingTook a bit of digging to find what shall be the character encoding for all telegrams, the national standards don't spell it out, but refer to EN-IEC 62056-21. EN-IEC 62056-21 is hard to get hold off (the official version is ridiculously expensive.) But I found a version of it on the web: EN-IEC 62056-21. What is the actual character set used for the data part of the telegram messages according to iec62056-21? This is spelled out in clause 5.5:
ISO 646:1991 IRV (international reference version) is basically the same as US-ASCII. (See also: wikipedia article on ISO/IEC 646, oder hier ein sehr übersichtliche Darstellung übersichtliche Darstellung ) Follows that ASCII is the proper choice. Use of Latin-1 (ISO-8859-1) decoding instead of ASCII decodingThe first 128 characters of latin-1 are identical with ASCII. The Latin-1 decoder is the most simple of all decoders: it is in fact 1 to 1 mapping of the data bytes. It seems harmless if we change the decoding to latin1. CRC checkInspection of the CRC check shows that it is working on the full 8 bits, therefore this is also fine. The CRC Check explicitly checks the part between the beginning '/' and the '!'. The parserIn fact the parser does not limit itself to the the part between the beginning '/' and the '!'. Meaning that we could add telegram lines after the checksum, which still would be processed or worse override telegram lines from the verified part. This is not good. I.e.no check here or elsewhere in the stages before that: for signature, parser in self.telegram_specification['objects'].items():
match = re.search(signature, telegram_data, re.DOTALL)Making it more robustSo to make things more robust we need to drop everything that comes after the checksum. My conclusion is that it would be:
|
|
Thanks for checking. Wondering why its actually running fine on my end for over a week in HomeAssistant with a few lines of changes to the integration to use the Q3D parser. |
|
Maybe the 255 thingy is not there in your setup with Home Assistant. Some copy paste error? |
|
Yes, your tests fail here as well. But this works: (reformatted) |
|
I found your samples under example_telegrams.py. They work allright. |
|
Yes, sorry formatting messed it up ... see above |
|
Same issue in your tests. Now they work. runs like a charm |
|
re "that anything past the CRC line is ditched before it is entered into the parsing stages" telegram_buffer.get_all() does excatly that before protocol.py hands it over to handle_telegram where the parser is actually called. Don't you think thats sufficient? |
|
FYI Here is a deeper look inside of the COM-1 box which is actually a WIZnet WIZ110SR. |
Yes, you are right. its a long time ago I looked at that part. I think that this in # we accepted 8-bit at transport level (e.g. tcp)
telegram_data = telegram.encode("latin1")
# we need to ensure 7-bit at telegram level (IEC 646 required in section 5.4 of IEC 62056-21)
telegram = telegram_data.decode("ascii")so I propose to remove it. |
|
| @@ -181,3 +181,19 @@ | |||
| obis.INSTANTANEOUS_CURRENT_L3: CosemParser(ValueParser(Decimal)), | |||
| } | |||
| } | |||
|
|
|||
|
|
|||
| Q3D = { | |||
There was a problem hiding this comment.
This is the last thing we need to do, I think. The problem is that the parser can not distinguish between ELECTRICITY_IMPORTED_TOTAL, LUXEMBOURG_ELECTRICITY_USED_TARIFF_GLOBAL, SWEDEN_ELECTRICITY_USED_TARIFF_GLOBAL, as they are exactly the same regarding the matching pattern used.
Same for LUXEMBOURG_ELECTRICITY_DELIVERED_TARIFF_GLOBAL, SWEDEN_ELECTRICITY_DELIVERED_TARIFF_GLOBAL, ELECTRICITY_EXPORTED_TOTAL.
To unbreak things, we have to implement your simplified proposal. You want to give it a go?
|
With this I had just deferred decoding to 7-bit from the receiving to the parsing stage, allowing 8-bit just when we receive data from the port. At this stage we only handle the telegram and prepare for parsing it. Decoding to 7-bit in handle_telegram will throw exceptions right away when the actual telegram doesn't comply to spec. Otherwise we are leaving it up to the parser and crc-check to find such errors. In case of Q3D we don't have a crc. Also this limits my pull request to just allow 8-bit on the transport layer surrounding 7-bit telegram, where the \xFF-issues are located. I would suggest to keep it. I'm happy to consolidate the obis codes, but would suggest to do that in a second pull request, as it will affect 5S and 5L. Are you fine with this and ready to merge? |
|
Fair enough. Can we then agree to the change below?
Could not push it to the Pull Request, so here is a diff: hanserik@famsrv1:~/projects/dsmr_parser$ git diff 26ac27c347c39d473c7f034eab2bdb295927d2df a04a7cc4c5974107b482cce55f48dd6c5e32d7f8
diff --git a/dsmr_parser/clients/protocol.py b/dsmr_parser/clients/protocol.py
index c2d032f..ca2b4eb 100644
--- a/dsmr_parser/clients/protocol.py
+++ b/dsmr_parser/clients/protocol.py
@@ -102,12 +102,16 @@ class DSMRProtocol(asyncio.Protocol):
def data_received(self, data):
"""Add incoming data to buffer."""
+
+ # accept latin-1 on the line, to allow for non-ascii padding
data = data.decode("latin1")
self._active = True
self.log.debug('received data: %s', data)
self.telegram_buffer.append(data)
for telegram in self.telegram_buffer.get_all():
+ # ensure actual telegram is ascii only
+ telegram = telegram.encode("latin1").decode("ascii")
self.handle_telegram(telegram)
def keep_alive(self):
@@ -134,10 +138,6 @@ class DSMRProtocol(asyncio.Protocol):
self.log.debug('got telegram: %s', telegram)
try:
- # we accepted 8-bit at transport level (e.g. tcp)
- telegram_data = telegram.encode("latin1")
- # we need to ensure 7-bit at telegram level (IEC 646 required in section 5.4 of IEC 62056-21)
- telegram = telegram_data.decode("ascii")
parsed_telegram = self.telegram_parser.parse(telegram)
except InvalidChecksumError as e:
self.log.warning(str(e))
|
|
@ndokter, looks good now. Can be merged. |
|
Would be nice also to bump up the version to 0.31 subsequently. But not sure what's needed for a release. ... I'm a github novice |
|
Leave that up to @ndokter. He is the master of this repo. We are only humble servants ;-) |
|
Heh i wish it was a standalone repo with multiple people having access. Just didnt find the motivation to set it up yet :) Thanks for your work, ive merged it and released it |
I run two EasyMeter Q3D for my main energy incl delivery of solar panel generated energy, the second one is a "private" one just to meter my main consumption by a heat pump. Both have IR interfaces which are bridged to ethernet TCP Port 5000 by COM-1 boxes from co-met.info. Unfortunatly the tcp stream from the main meter includes (padding?) \xFF which throws off the decode(ascii), so I set it to errors="ignore". Hope that doesn't create any other issues.