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

Added support for 5 Channel S0 USB Pulse meter (original: 5-kanaals S0 Pulse Meter op USB) #596

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

Lurchi70
Copy link

@Lurchi70 Lurchi70 commented Jun 28, 2023

Implementation a new device protocol: sos_s0

The 5 Channel S0 USB Pulse meter is a device based on Arduino Micro. It collects and buffers S0 impulse from up to five S0 devices and sends the retrieved S0-IMpulses within a 10 second timeframe to the serial port (e.g. /dev/ttyACM0) available via standard USB driver. The protocol is documented by the vendor.
The device is available here: https://www.sossolutions.nl/5-kanaals-s0-pulse-meter-op-usb

This change adds a new protocol "sos_s0" to vzlogger. The implementation opens the device port, applies the hardcoded baud rate and parity setting (9600/7/n/1) and reads the data from the serial device.
The read data is provided to volkzaehler middleware by the usual ways.

Adding support for S0 pulse meter from https://www.sossolutions.nl
"5-kanaals S0 Pulse Meter op USB"
Adding support for S0 pulse meter from https://www.sossolutions.nl
"5-kanaals S0 Pulse Meter op USB"
# Conflicts:
#	include/protocols/MeterSOS_S0.hpp
#	src/protocols/MeterSOS_S0.cpp
@J-A-U
Copy link
Collaborator

J-A-U commented Jun 29, 2023

Interesting hardware you got there.

You might add this also to ./etc/vzlogger_generic.schema.json , for generated configuration.

++ aligned source code with clang-format
++ modified vzlogger_generic.schema.json for generated configuration
++ added send_zero parameter to sample conf file
Copy link
Author

@Lurchi70 Lurchi70 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now. Please Review.

@J-A-U J-A-U requested a review from r00t- June 30, 2023 16:11
@J-A-U
Copy link
Collaborator

J-A-U commented Jun 30, 2023

@r00t- bump version?

@r00t-
Copy link
Contributor

r00t- commented Jul 2, 2023

thanks for your contribution,
nice to see vzlogger used as a platform to support new meters,
will try to review the code next week.

@Lurchi70
Copy link
Author

Lurchi70 commented Jul 13, 2023 via email

@r00t-
Copy link
Contributor

r00t- commented Sep 17, 2023

@Lurchi70:
is it a priority for you to have this merged?
imho, as long as there are few users, they can/will just find the MR and use the code,
and also it would be good to have more than one user trying a meter before it gets merged.
(this is the same for a meter i implemented myself: #480 )

if you want to have it merged, i'd have some small requests:

  • you based your meter on copied code from MeterD0, which is fine in itself, but i would suggest to first commit an unmodified copy, so it's easy to see (and extract) your actual new code. (as i did in https://github.com/volkszaehler/vzlogger/pull/480/commits => 2a33a92 + 657b0ad ), i could assist if help is needed.
  • consider those weird stray comments like "RW: added ack" or "snip from kernel end" that were copied along (would be fixed by the above suggestion)
  • (in the long run, we should extract the serial-port handling code into some shared functions instead of duplicating the code in every serial meter, but that doesn't need to be done here.)
  • (probably too much, and i didn't do it myself either - if few people have the hardware, that would make it even more important to have units tests for the code, so we can avoid it breaking unexpectedly with compiler updates or refactorings.)

* Plaintext protocol of S0 Pulse meter devices
* --> https://www.sossolutions.nl/5-kanaals-s0-pulse-meter-op-usb
*
* The device is a USB s0 logger device.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe just a little bit more description of the communication, at least "sending a text-based protocol over a usb-connected uart interface"?
maybe the usb-IDs used by the device or a udev rule to identify it.

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

Successfully merging this pull request may close these issues.

3 participants