ndokter / dsmr_parser

Library to parse Dutch Smart Meter Requirements (DSMR) telegrams.

MIT License

110 stars 64 forks source link

Optional keep alive monitoring for TCP/IP connections #73

Closed hogend closed 3 years ago

hogend commented 3 years ago

Since dsmr-parser is listen-only, it will not notice interrupted connections and DSMR device restarts. The connection will be reset after an (optional) keep-alive interval if no messages were received from the device.

codecov-io commented 3 years ago

Codecov Report

Merging #73 (7453534) into master (3dc77a8) will decrease coverage by 0.69%. The diff coverage is 37.50%.

@@            Coverage Diff             @@
##           master      #73      +/-   ##
==========================================
- Coverage   61.40%   60.70%   -0.70%     
==========================================
  Files          15       15              
  Lines         557      570      +13     
  Branches       66       68       +2     
==========================================
+ Hits          342      346       +4     
- Misses        210      219       +9     
  Partials        5        5

Impacted Files	Coverage Δ
dsmr_parser/__main__.py	`0.00% <0.00%> (ø)`
dsmr_parser/clients/protocol.py	`46.59% <40.00%> (-2.75%)`	:arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3dc77a8...7453534. Read the comment docs.

hogend commented 3 years ago

This change would allow the home-assistant component to add a keep-alive interval of, say 60s, to the "create_tcp_dsmr_reader" entry point. If the DSMR device restarts or the connection between HA and the device is interrupted, the connection will be terminated and restarted if no messages were received in the keep-alive interval. Note that the "**kwargs" argument is needed to trickle down the keep-alive-interval parameter to Protocol. init

RobBie1221 commented 3 years ago

@hogend I think some test code is needed to let build pass. Are you up for making that?

hogend commented 3 years ago

I guess it would be possible to satisfy the coverage test by adding a test with a keep_alive_interval specified. I have no experience with the test framework, though.Also, I do not see a way to automatically test the dynamic behaviour, e.g. to create a server, connect to the server, restart the server and check that the connection is re-established.

hogend commented 3 years ago

@hogend I think some test code is needed to let build pass. Are you up for making that?

@RobBie1221: Please, have a look at the last commit.

RobBie1221 commented 3 years ago

At least Codecov is happy :-)

@ndokter Are you fine with this change? Many users are experiencing issues with dropped connections within Home Assistant, as the library only receives data it's quite impossible to detect dropped connections without a watchdog.

ndokter commented 3 years ago

Yeah looks good! Thanks

RobBie1221 commented 3 years ago

Thanks!

Could you make a new release as well? Then we can bump the version in Home Assistant.

ndokter commented 3 years ago

I've built a new version, but haven't tested it myself

RobBie1221 commented 3 years ago

Thanks, I'll test it inside my Home Assistant development setup