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

Feature request: implement CRC check #10

Closed
ThinkPadNL opened this issue Oct 26, 2016 · 23 comments
Closed

Feature request: implement CRC check #10

ThinkPadNL opened this issue Oct 26, 2016 · 23 comments

Comments

@ThinkPadNL
Copy link

As far as i can see, the code doesn't have a CRC check to look if the incoming telegram is correct. However, the modern >=DSMR4 meters include a CRC at the end of the telegrams. With the CRC16 these can be checked, see also: https://gathering.tweakers.net/forum/list_messages/1704755

Would be nice if this would be implemented, to prevent false data being imported.

@nrocco
Copy link
Owner

nrocco commented Oct 26, 2016

Hi @ThinkPadNL,

Interesting. This would be a nice addition to smeterd.
Can you provide some sample packets that include a CRC in the end so I have some data to work with? My meter does not provide such CRC checksums, unfortunately.

@jvhaarst
Copy link

I'm not at home right now, so I can't give you an example telegram with a CRC, but I do have some code that could help here :
https://github.com/jvhaarst/DSMR-P1-telegram-reader/blob/master/telegram_from_serial.py#L114

@ThinkPadNL
Copy link
Author

@nrocco I'm willing to provide you with some sample data, but i don't want to spread my smartmeter' serial number everywhere on the net. But to do a CRC check on the samples you probably need the untouched telegram i guess?

Please let me know how i can assist in this.

@nrocco
Copy link
Owner

nrocco commented Nov 6, 2016

@ThinkPadNL I agree with you, let us avoid spreading serial numbers everywhere on the net.

How about posting the complete packet here with your serial number anonymised and share the latter via twitter direct message?

@ThinkPadNL
Copy link
Author

@nrocco That's okay, what is your Twitter username?

@nrocco
Copy link
Owner

nrocco commented Nov 7, 2016

My twitter username is nrocco.

@ThinkPadNL
Copy link
Author

ThinkPadNL commented Nov 7, 2016

@nrocco I followed you (Fr...), please follow me back so i can send you a PM.

@ThinkPadNL
Copy link
Author

Any progress so far :D ?

@nrocco
Copy link
Owner

nrocco commented Nov 14, 2016

No much progress I am afraid. I have tried several python crc libraries such as crcmod but so far I did not manage to calculate the checksum correctly.

Do you have any working code?

@ThinkPadNL
Copy link
Author

ThinkPadNL commented Nov 14, 2016

Yes, i saw this code: https://gathering.tweakers.net/forum/list_message/48049109#48049109
His code code mentions 'crc' in several places. He says the CRC is working...

I also found this reference about the CRC16 that is used for the P1 packets: https://github.com/mhe/dsmr4p1/blob/master/dsmr4p1.go#L18

Maybe the DSMR specs can also be useful: http://files.domoticaforum.eu/uploads/Smartmetering/DSMR%20v4.0%20final%20P1.pdf
See paragraph 5.2.

Have you also seen the code that jvhaarst mentions? I see some crc things in his code

@jvhaarst
Copy link

I just checked my code by running it for the first time in months, and my code is able to check the checksum.
Calculating it is a bit annoying, but the code should help to get it working for you.
If not, just ask.

@nrocco
Copy link
Owner

nrocco commented Nov 14, 2016

@ThinkPadNL, I will check the resources you provided.

@jvhaarst Weird. I did look at your code (I am now using the same python crc package that you are using) but could not get it to work.

@jvhaarst
Copy link

@nrocco In the virtual environment in which I run it at the moment, pip freeze says :

argparse==1.2.1
crcmod==1.7
pyserial==3.0.1
wsgiref==0.1.2

@nrocco
Copy link
Owner

nrocco commented Nov 14, 2016

Similar for me:

crcmod==1.7
pycli-tools==2.0.2
pyserial==3.2.1
wheel==0.24.0

I have reason to believe it has something to do with smeterd converting the binary data to utf-8 in the while loop and stripping line endings.

This is something you do not do in https://github.com/jvhaarst/DSMR-P1-telegram-reader/blob/master/telegram_from_serial.py#L114

Instead you keep the binary data from as-is

telegram = telegram + telegram_line

I will try to verify my assumptions.

@jvhaarst
Copy link

Reading through my code it indeed looks like the whole telegram needs to be crc'ed, so including the newlines.

@nrocco
Copy link
Owner

nrocco commented Nov 23, 2016

I am making slow progress on this feature but I am almost at the point of sumbmitting a pull request.

Thanks for your patience, so far.

@ThinkPadNL
Copy link
Author

Good to hear! Let me know if the CRC has been implemented 👍

@nrocco
Copy link
Owner

nrocco commented Nov 24, 2016

@ThinkPadNL I just submitted a pull request that introduces the crc check, see #14

I would appreciate it if you could help me test this against a real smart meter (mine does not include checksums, unfortunately).

If you could test it against python 2.x and python 3.x, that would be great.

@ThinkPadNL
Copy link
Author

Nice!

Like i said, i have very little programming skills. I want to help you test the things you said, but you have to explain me how haha ;)

Where do i start now?

@nrocco
Copy link
Owner

nrocco commented Nov 27, 2016

@ThinkPadNL no worries, I did some testing against my smart meter from python 2 and python 3. I was comfortable enough to create a new release 2.6.1 which is already uploaded to pypi.

Packets without a valid crc no throw an exception.

@nrocco nrocco closed this as completed Nov 27, 2016
@ThinkPadNL
Copy link
Author

ThinkPadNL commented Nov 27, 2016

It works!

pi@raspberrypi:~/p1test/smeterd$ python -m smeterd read-meter --baudrate 115200
Time                      2016-11-27 20:12:30.407350
Total kWh High consumed   1945448
Total kWh Low consumed    1920500
Total gas consumed        1415245
Current kWh tariff        1
pi@raspberrypi:~/p1test/smeterd$

I also managed to rewrite the script i am using to put the data into InfluxDB (and then graph it with Grafana). This is the old version, this is the new version. Probably really ugly, but i am quite proud of myself already that i was able to mangle your new code into the old script (that i didn't make myself, i have almost no Python skills).

P.S. Maybe you should also mention the --baudrate option on the intro page of this repo. The 9600 baudrate is for old meters, all new DSMR4 meters use 115200 :)

@nrocco
Copy link
Owner

nrocco commented Nov 28, 2016

Good to hear!

You can expect more pull requests in the next couple of days to:

  • introduce more command line flags and
  • expose more information in the output
  • add more output formats such as --json

I will take your feedback into account.

@dennissiemensma
Copy link

dennissiemensma commented Dec 25, 2016

Hi @nrocco, yesterday @ThinkPadNL requested CRC as well in dsmrreader/dsmr-reader#188 and pointed me to your app.
Your CRC code is a good example of how this DSMR check works, so I just want to say thank you for sharing this on Github. :]

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

No branches or pull requests

4 participants