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

Verify telegrams' CRC #188

Closed
dennissiemensma opened this issue Dec 24, 2016 · 14 comments
Closed

Verify telegrams' CRC #188

dennissiemensma opened this issue Dec 24, 2016 · 14 comments
Milestone

Comments

@dennissiemensma
Copy link
Member

dennissiemensma commented Dec 24, 2016

The telegrams should be verified by checking their CRC. I'm planning to use #154 but that also lacks CRC checks, for now.

I should either re-check later or write it myself.

[Originally reported by @ThinkPadNL]

@dennissiemensma
Copy link
Member Author

Requested via #187

@ThinkPadNL
Copy link

ThinkPadNL commented Dec 25, 2016

Too bad it doesn't use CRC yet. I have had some incorrect data a few times with the script i am using now, so it is definitely a recommendation to use, if you have a modern enough meter (>= DSMR4). I am currently using some other library and he has implemented CRC checking: nrocco/smeterd#10
Maybe you can find some inspiration there? The method used is crc16.

dennissiemensma added a commit that referenced this issue Dec 25, 2016
@dennissiemensma dennissiemensma modified the milestones: 1.5, 1.6 Dec 25, 2016
@dennissiemensma
Copy link
Member Author

@ThinkPadNL a lot of thanks to you for providing the info! I managed to have the CRC checksum working now.

I will run it at home for a while and merge fbaed6c back to the main development branch for the upcoming release when no problems occur.
That release is scheduled for mid January, so this feature won't be available right away.

Optionally, if you'd like to be listed on the 'thanks' page, please leave a note in #180.

@ThinkPadNL
Copy link

ThinkPadNL commented Dec 25, 2016

Haha, you added that very quickly! Great!

Nah, i don't have to be listed as i'm not the author of the code i linked to. It is made by 'nrocco' from that GitHub page, so if someone is to be deserving credits, it should be him :)

dennissiemensma added a commit that referenced this issue Dec 25, 2016
@dennissiemensma
Copy link
Member Author

Sure, I will mention him and drop him a message.

I won't list you if you do not want to. Please note that I do not list people for providing other's code, but for bringing the ideas and pointing out any issues. Just so you know. :]

@dennissiemensma
Copy link
Member Author

Code is running fine for few hours now, although no bad telegrams were seen.

The tests pass as well. So merged back to development for v1.5.

Thanks again for requesting this feature.

@dennissiemensma
Copy link
Member Author

@ThinkPadNL the v1.5 release is out, containing this issue.

Please mind:

Also note that upgrading requires you to cd into the project root and activate the VirtualEnv, after switching to the dsmr user. The installation guide hinted that before, but it's highlighted more clearly now.

@ThinkPadNL
Copy link

I installed your software today, now that it has CRC-checking (i have a 5m long cable from my meter cabinet to my server, so i needed CRC to ignore false data. #187 would let me remove the long cable ).

Where can i see how many corrupt packages it ignored?

@dennissiemensma
Copy link
Member Author

You can see it on the Statistics page, but it only displays the counter when any readings were discarded. The counter can also be seen in the Admin (Configuration) at the Meter statistics menu item.

It contains several fields, and one of them is rejected_telegrams: Number of rejected telegrams due to invalid CRC checksum (or translated). It should always be listed there.

@ThinkPadNL
Copy link

ThinkPadNL commented Jan 1, 2017

Rejected telegrams: 0. Software is running for about 40 minutes now.
I guess that it was a wise decision to use some twisted CAT5E-cable to create a P1-USB cable :)

Will keep monitoring the value to conclude if it really works. With my own script i sometimes had incorrect data due to corrupt telegrams.

Btw, what is the preferred language on this Github repo? The project is written for Dutch smartmeters, so i am currently wondering why i started typing in English anyway haha :)

@dennissiemensma
Copy link
Member Author

Cool, I'd love to see any reading fail though. I could only reproduce a failed CRC manually while writing mocked tests so far. Not in production use.

Regarding the language, you can pick either English or Dutch. I'm actually surprised, and also a bit honored, that people just seem to start talking English here anyway. Good ol' Dutch habbits I guess.

@ThinkPadNL
Copy link

ThinkPadNL commented Jan 1, 2017

There it is:
Rejected telegrams: 1

Graphs are still looking good (i don't see any enormous usage as i had with my own script), so i can conclude (with this real life example) that the verification of the telegram checksum is working correctly!

(2760 telegrams stored and 1 ignored, according to the /statistics page)

@dennissiemensma
Copy link
Member Author

Great, thanks for sharing. Good to see that it actually helps in someone's production as well.

@dennissiemensma
Copy link
Member Author

Mocht je trouwens nog benieuwd zijn naar het foutieve telegram, volgens mij kan je hem hiermee vinden:

grep InvalidTelegramChecksum /var/log/supervisor/dsmr_datalogger.log -C 20

En anders zou het een van de twee foutmeldingen in deze code moeten zijn (vermoedelijk de laatste).

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

No branches or pull requests

2 participants