Skip to content

[WIP] Readd serial support#29

Draft
tykling wants to merge 111 commits intoRichiH:mainfrom
tykling:readd_serial_support
Draft

[WIP] Readd serial support#29
tykling wants to merge 111 commits intoRichiH:mainfrom
tykling:readd_serial_support

Conversation

@tykling
Copy link
Copy Markdown

@tykling tykling commented Nov 3, 2021

As discussed on IRC. Work in progress, warning: TCP untested.

The PR also adds config for a couple of electricity meters I've been testing with, Carlo Gavazzi EM111 and EM340.

SuperQ and others added 5 commits December 8, 2020 10:27
Remove the modbus exporter meta-metrics that duplicate data from
the scrape information. In order to differentiate between different
failures, use http status codes instead.

Signed-off-by: Ben Kochie <superq@gmail.com>
Trying to help other people dodge some of the hurdles I stumbled over
…port tested, but I might have broken TCP support in the process
…king Modbus over RS485. Not all registers are included, but the important ones are.
@tykling
Copy link
Copy Markdown
Author

tykling commented Nov 3, 2021

@bastischubert any chance you can test if TCP support still works with this patch? I only have RS485 devices here with me. @RichiH said on IRC that you might be able to help :)

@bastischubert
Copy link
Copy Markdown
Collaborator

I'll test on monday when i'm back from vacation☺️

@rfc1036
Copy link
Copy Markdown

rfc1036 commented Nov 3, 2021

You can use https://github.com/3cky/mbusd as a serial to RTU converter, I have been using it for a while to use modbus_exporter with a cheap USB RS-485 adapter. Thank you for working on serial support!

…king Modbus over RS485. Not all registers are included, but the important ones are.
Copy link
Copy Markdown
Collaborator

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

Some minor nits, otherwise seems sane.

@tykling
Copy link
Copy Markdown
Author

tykling commented Nov 8, 2021

This functionality is broken when using more than one subtarget on the same target. The problem is that serial communication is, well, serial, and prometheus might scrape two targets simultaneously, confusing the modbus code.

I haven't had any luck figuring out how to make sure only one thing happens at a time on the rs485 serial bus. Does anyone have input on how to achieve this? I was hoping MaxRequestsInFlight: 1 on the promhttp handler at the very the end of modbux_exporter.go would help, but it seems a new handler is made for each request by the router/mux, so that did nothing.

Any input would be appreciated. Otherwise I will sadly have to close the PR since I have no idea how to fix it.

@RichiH
Copy link
Copy Markdown
Owner

RichiH commented Nov 8, 2021

@SuperQ can you help with making certain that sequentiality is ensured?

Does this appear per target only or would we hit the same with a lot of different targets at once? Global locks or sequencing is meh. If we end up in that situation, we would need to emit some metrics around this.

@tykling
Copy link
Copy Markdown
Author

tykling commented Nov 8, 2021

The issue is only per serial bus/device (for example /dev/ttyUSB0). The serial device is called target in the modbus_exporter code. So only one subtarget (power meter) per target (serial device) at a time.

A lot of different targets at once is fine (as in, different serial devices /dev/ttyUSB0 and /dev/ttyUSB1 and /dev/ttyUSB2 can be hit at the same time).

When using relabelling the targets in prometheus.yml become sub_targets in modbus_exporter, so it is a bit confusing. But I think the above should be clear, see also the log snippet in the next comment :)

@tykling
Copy link
Copy Markdown
Author

tykling commented Nov 8, 2021

This log snippet shows the issue - all sub_target (meters) are on /dev/ttyUSB0 (the serial bus) and prometheus is scraping sub_target 2 and 7 too close to eachother, so the underlying modbus code gets confused:

INFO[109961] got scrape request for module 'carlo_gavazzi_em_et_3xx_series' target '/dev/ttyUSB0' and sub_target '5' source="modbus_exporter.go:130"
INFO[109968] got scrape request for module 'carlo_gavazzi_em_et_1xx_series' target '/dev/ttyUSB0' and sub_target '8' source="modbus_exporter.go:130"
INFO[109970] got scrape request for module 'carlo_gavazzi_em_et_3xx_series' target '/dev/ttyUSB0' and sub_target '1' source="modbus_exporter.go:130"
INFO[109972] got scrape request for module 'carlo_gavazzi_em_et_1xx_series' target '/dev/ttyUSB0' and sub_target '10' source="modbus_exporter.go:130"
INFO[109989] got scrape request for module 'carlo_gavazzi_em_et_3xx_series' target '/dev/ttyUSB0' and sub_target '4' source="modbus_exporter.go:130"
INFO[109992] got scrape request for module 'carlo_gavazzi_em_et_1xx_series' target '/dev/ttyUSB0' and sub_target '7' source="modbus_exporter.go:130"
INFO[109992] got scrape request for module 'carlo_gavazzi_em_et_3xx_series' target '/dev/ttyUSB0' and sub_target '2' source="modbus_exporter.go:130"
ERRO[109994] failed to scrape metrics for module 'carlo_gavazzi_em_et_3xx_series,' target '/dev/ttyUSB0', sub_target '2': metric 'emet3xx_phase_voltage_volts', address '300000': serial: timeout source="modbus_exporter.go:148"
ERRO[109994] failed to scrape metrics for module 'carlo_gavazzi_em_et_1xx_series,' target '/dev/ttyUSB0', sub_target '7': metric 'emet1xx_consumed_kvarh_total', address '300018': serial: timeout source="modbus_exporter.go:148"
INFO[109995] got scrape request for module 'carlo_gavazzi_em_et_1xx_series' target '/dev/ttyUSB0' and sub_target '9' source="modbus_exporter.go:130"
INFO[110004] got scrape request for module 'carlo_gavazzi_em_et_1xx_series' target '/dev/ttyUSB0' and sub_target '6' source="modbus_exporter.go:130"
INFO[110020] got scrape request for module 'carlo_gavazzi_em_et_3xx_series' target '/dev/ttyUSB0' and sub_target '5' source="modbus_exporter.go:130"
INFO[110028] got scrape request for module 'carlo_gavazzi_em_et_1xx_series' target '/dev/ttyUSB0' and sub_target '8' source="modbus_exporter.go:130"
INFO[110030] got scrape request for module 'carlo_gavazzi_em_et_3xx_series' target '/dev/ttyUSB0' and sub_target '1' source="modbus_exporter.go:130"
INFO[110032] got scrape request for module 'carlo_gavazzi_em_et_1xx_series' target '/dev/ttyUSB0' and sub_target '10' source="modbus_exporter.go:130"
INFO[110049] got scrape request for module 'carlo_gavazzi_em_et_3xx_series' target '/dev/ttyUSB0' and sub_target '4' source="modbus_exporter.go:130"
INFO[110052] got scrape request for module 'carlo_gavazzi_em_et_1xx_series' target '/dev/ttyUSB0' and sub_target '7' source="modbus_exporter.go:130"
INFO[110052] got scrape request for module 'carlo_gavazzi_em_et_3xx_series' target '/dev/ttyUSB0' and sub_target '2' source="modbus_exporter.go:130"
ERRO[110052] failed to scrape metrics for module 'carlo_gavazzi_em_et_1xx_series,' target '/dev/ttyUSB0', sub_target '7': metric 'emet1xx_firmware_revision', address '300771': resource temporarily unavailable source="modbus_exporter.go:148"
ERRO[110054] failed to scrape metrics for module 'carlo_gavazzi_em_et_3xx_series,' target '/dev/ttyUSB0', sub_target '2': metric 'emet3xx_phase_voltage_volts', address '300000': serial: timeout source="modbus_exporter.go:148"
INFO[110055] got scrape request for module 'carlo_gavazzi_em_et_1xx_series' target '/dev/ttyUSB0' and sub_target '9' source="modbus_exporter.go:130"
INFO[110064] got scrape request for module 'carlo_gavazzi_em_et_1xx_series' target '/dev/ttyUSB0' and sub_target '6' source="modbus_exporter.go:130"

@tykling
Copy link
Copy Markdown
Author

tykling commented Nov 13, 2021

OK, after a bit of fiddling this actually works really well now. I had to update the goburrow/modbus dependency because I hit an old bug they fixed since then. I solved the sequentiality requrement by wrapping the scrape in a mutex for serial scrapes. This means if one thread is already using the serial bus another thread will have to wait. I've added two new metrics to track the mutex stuff, a counter to total how many seconds are spent waiting for a mutex lock, and a gauge to show how many are waiting right now.

This leave one open issue, I don't have a system with two serial busses right now, but the mutex should be serial bus specific (there is no need to make a scrape on serial bus /dev/ttyUSB1 wait for /dev/ttyUSB0 to finish).

Before I dig into solving that (I do not have any systems with multiple serial adapters attached to seperate rs485 busses handy to test) I would like to request a review to make sure what I have now will be accepted when I finish the PR. Maybe @SuperQ can take another look? Thanks!

@v-zhuravlev
Copy link
Copy Markdown

v-zhuravlev commented Jan 30, 2022

Hi, thanks for this feature!
I tried to test it, and facing one issue with this config:

modules:
    # https://www.owen.ru/uploads/re_mv110-224.2a_1822.pdf
  - name: "owen_mv110"
    protocol: 'serial'
    baudrate: 9600
    databits: 8
    parity: "N"
    stopbits: 1
    timeout: 3000
    metrics:
      - name: "mv110_sensor"
        help: "represents the current reading of analogue sensor"
        labels:
          input: "1"
        address: 300004
        dataType: float32
        endianness: big
        metricType: gauge
      - name: "mv110_sensor"
        help: "represents the current reading of analogue sensor"
        labels:
          input: "2"
        address: 300010
        dataType: float32
        endianness: big
        metricType: gauge

It successfully retrieves the metric only if SINGLE metric is defined in modbus.yml. If I add one more metric, i'm getting a bunch of various errors, such as, timeout, CRC error or that another device (id=4) responded instead of actual one with sub_module=32. It looks to me that two requests are sent to serial port at the same time and so collision happens? Maybe I'm wrong.

Like so:

failed to scrape target '/dev/ttyUSB0' sub_target '32' with module 'owen_mv110': failed to scrape metrics for module 'owen_mv110', target '/dev/ttyUSB0', sub_target '32': metric 'mv110_sensor', address '300010': modbus: response slave id '4' does not match request '32'
failed to scrape target '/dev/ttyUSB0' sub_target '32' with module 'owen_mv110': failed to scrape metrics for module 'owen_mv110', target '/dev/ttyUSB0', sub_target '32': metric 'mv110_sensor', address '300010': serial: timeout
failed to scrape target '/dev/ttyUSB0' sub_target '32' with module 'owen_mv110': failed to scrape metrics for module 'owen_mv110', target '/dev/ttyUSB0', sub_target '32': metric 'mv110_sensor', address '300010': resource temporarily unavailable

HW setup should be fine, since, I can poll multiple metrics using zabbix https://github.com/v-zhuravlev/libzbxmodbus and also I can get data using modpoll utility.

Sample request I'm using:
http://192.168.15.200:9602/modbus?target=/dev/ttyUSB0&sub_target=32&module=owen_mv110

@tykling
Copy link
Copy Markdown
Author

tykling commented Jan 31, 2022

sounds like you might be scraping faster than your serial bus or devices can keep up, how often are you scraping and how long does a scrape take and how many targets do you have? remember, a serial bus is serial :)

I haven't forgotten about this patch and I will get back to it soon, I am currently pressed for time

tykling added 26 commits May 22, 2023 20:47
… error, only mutex lock for serial scrapes, and track some metrics around mutex locking
…fix a few copypaste and variable naming errors
… are any, and readd scrape request logline removed by mistake
…ecase is when using these meters through a modbus RTU to modbus TCP gateway/proxy
…te registers is used for the integer and decimal parts of the register, add cagaemet_consumed_precision_kwh_total metric using this type of register in modbus.yml
@RichiH
Copy link
Copy Markdown
Owner

RichiH commented Dec 5, 2023

(Just to track this, I keep clicking the "Approve and Run" on the tests)

@tykling what's the status on your end? Does serial work in production?

@bastischubert do you currently have hardware to test TCP against?

@RichiH
Copy link
Copy Markdown
Owner

RichiH commented Dec 5, 2023

Hmm, in this round of tests, even more fail than the last round. See above for details, @tykling

@tykling
Copy link
Copy Markdown
Author

tykling commented Dec 5, 2023

hey :)

Yes, serial works in production, even with multiple serial ports (without parallel scrapes messing anything up). I am very happy with it actually. I also tested TCP a while back and it works well.

But this PR is a mess. I fucked up a merge a while back so the branch has weird history with a bunch of commits that are not relevant. I have been meaning to close it and open a new "fresh" PR.

I also have a bunch of questions about the PR and the general direction you want to take with the project.

  • a while back you said that you wanted modbus.yml to be like the snmp.yml file in the snmp_exporter - meaning you wanted it to be "ready to run" and slowly grow as new devices are added. For this reason I have included the devices I use with modbus_exporter in modbus.yml in this PR, but I don't see any other devices added. Did I misunderstand something? Do you want device registers in modbus.yml or not?
  • The devices I monitor have quirks which required new features in the code. The offset feature where a value is offset in a combined register. And a few days ago I had to add a new weird datatype which may or may not exist elsewhere, where a float64 is put into two int32 fields. The first int32 field has the integer part of the number and the next has the decimals. But the MSW of the decimals is always 0 so it requires some fiddling. This feels a bit weird to put into the exporter because I doubt many other devices use such weird registers. But otherwise it wont work. Thoughts? Do you want these two features in a different PR?

I think back ages ago when I first forked this there was not much in the way of tests so I haven't written any. But I will take a look at the failing tests and see what I can do. I am not very experienced with Go, this has been my very first real adventure with it in fact :)

@RichiH
Copy link
Copy Markdown
Owner

RichiH commented Dec 5, 2023

Nice to hear it's working in prod!

Feel free to open a new PR, or clean up locally and force-push into this one. Both works.

Yes, we want to collect more default configs, but there's not a huge inrush of configs... Feel invited to add everything you have.

Modbus has a lot of weird warts -- As long as there's a sane way to express those quirks in declarative configuration, I'd err on the side of supporting the weird stuff.

Any help with the tests, or new tests, appreciated. This is more a work of love and passion by a bunch of people without the hardware to actually test the software, so every bit helps.

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.

10 participants