mtdcr / pysml

Python library for EDL21 smart meters using Smart Message Language (SML)
MIT License
16 stars 8 forks source link

fixed bug with 1 byte CRC16 #3

Closed kheinz57 closed 3 years ago

kheinz57 commented 3 years ago

I have seen a bug on CRC calculation. Whenever the calculated 16Bit CRC is less than 256 it will consume 1 Byte only. So instead of a 63 XX XX it will be a 62 XX only. But the buffer to calculate the CC ends always 4 bytes before the end of the 76 section. In case the CRC is serialized with 1 byte only this cutting off one byte of the data which has to be used to calculate the CRC. There might be a better solution but it seems to work here. Example: With 1 Byte CRC16: .....62 01 62 02 00.... <-- This was causing a problem before. With 2 Byte CRC16: .... 62 01 63 81 d8 00 ...

I have realized this while using EDL21 within home assistant together with a Weidmann Electronics USB IR Reader on an ISK MT631 smart meter

mtdcr commented 3 years ago

Thank you, @kheinz57! I took your idea and modified it a little bit (https://github.com/mtdcr/pysml/commit/79894a87b21776e81b4ad6971a8773dbcf5619b1, branch fix-crc16). Can you please check whether it still works for you?

I omitted the try-except statement for now, because I don't want it to fail silently.

kheinz57 commented 3 years ago

Hi @mtdcr

How can I replace the systems sml lib with the patched version? I have run sudo python3 setup.py but it seems like I still use the old version in the system. A pip3 unsintall and pip3 insatll . did not work either.

Any ideas? Heinz

mtdcr commented 3 years ago

You could just overwrite the file manually.

I think Home Assistant keeps installing version 0.0.3 on startup. You could edit edl21/manifest.json to remove the version constraint, as another option.

kheinz57 commented 3 years ago

Failed to install it with hassio. But my test script showes a clear differenc between the original version and your patch. You patch seem to solve the problem as good as mine (and does fit better into the software I have to admit). That's the script I was using to test for about 10min. It usually failes within <1min on the old version.

#!/usr/bin/env python3
#
# Copyright (c) 2019 Andreas Oberritter
#
# Permission is hereby granted, free of charge, to any person obtaining a copy
# of this software and associated documentation files (the "Software"), to deal
# in the Software without restriction, including without limitation the rights
# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
# copies of the Software, and to permit persons to whom the Software is
# furnished to do so, subject to the following conditions:
#
# The above copyright notice and this permission notice shall be included in
# all copies or substantial portions of the Software.
#
# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
# THE SOFTWARE.
#

import asyncio
import logging
from sys import argv
from sml import SmlSequence, SmlGetListResponse
from sml.asyncio import SmlProtocol

logging.basicConfig(level=logging.INFO)
logging.basicConfig(filename='sml-tty.log', level=logging.DEBUG)
logger = logging.getLogger(__name__)

class SmlEvent:
    def __init__(self):
        self._cache = {}

    def event(self, message_body: SmlSequence) -> None:
        assert isinstance(message_body, SmlGetListResponse)
        for val in message_body.get('valList', []):
            name = val.get('objName')
            if name:
                if self._cache.get(name) != val:
                    self._cache[name] = val
                    print(val)

def main(url):
    handler = SmlEvent()
    proto = SmlProtocol(url)
    proto.add_listener(handler.event, ['SmlGetListResponse'])
    loop = asyncio.get_event_loop()
    loop.run_until_complete(proto.connect(loop))
    loop.run_forever()

if __name__ == '__main__':
    if len(argv) != 2:
        print('usage: %s socket://127.0.0.1:51945\n' % argv[0] +
              '   or: %s /dev/ttyS0' % argv[0])
        exit(1)

    main(argv[1])
    exit(0)
dannerph commented 3 years ago

I can confirm that this PR fixes the CRC16 error, the code on the branch fix-crc16 is not working for me.

mtdcr commented 3 years ago

@dannerph I've released version 0.0.4 with this fix, because I couldn't reproduce the described behavior with the logs you provided by mail, and I think it shouldn't make things worse than before for you, at least, while at the same time it works for me and @kheinz57. Let's continue chasing your problem in #2.