Skip to content

generic modbus driver#1052

Merged
jimklimov merged 27 commits intonetworkupstools:masterfrom
dtsecon:master
Sep 20, 2021
Merged

generic modbus driver#1052
jimklimov merged 27 commits intonetworkupstools:masterfrom
dtsecon:master

Conversation

@dtsecon
Copy link
Copy Markdown
Contributor

@dtsecon dtsecon commented Jun 24, 2021

This is a generic modbus driver expected to work with contact (direct) signal UPS devices, connected via modbus RIO (remote I/O) either serial or TCP/IP. The driver has been tested against PULS UPS (model UB40.241) via MOXA ioLogikR1212 (RS485) and ioLogikE1212 (TCP/IP).

The PULS UPS UB40.241 supports the following signals:

  • Ready contact (DO) <--> HB
  • Buffering contact (DO) <--> OL | OB
  • Battery-low (DO) <--> LB
  • Replace Battery (DO) <--> RB
  • Inhibit (DI) <--> FSD

DO and DI assumes device perspective

The driver's concept is to map the UPS states (as defined in nut) onto UPS contacts' states

The driver has an extended configuration interface implemented using variables defined in ups.conf.

[generic_modbus]
# info parameters
driver = generic_modbus
port = /dev/ttyUSB0
# port = 192.168.2.104
# port = 192.168.2.104:1502
desc = "generic ups driver"
# device info
device_mfr = "PULS"
device_model = "UB40.241"
# serial settings
ser_baud_rate = 9600
ser_parity = N
ser_data_bit = 8
ser_stop_bit = 1
modbus slave id
rio_slave_id = 5
# UPS signal state attributes
OB_addr = 0x0
OB_regtype = 1
OB_noro = 0
LB_addr = 0x1
LB_regtype = 1
HB_addr = 0x2
HB_regtype = 1
RB_addr = 0x3
RB_regtype = 1
# CHRG_addr = 0x0
# CHRG_regtype = 1
# CHRG_noro = 1
DISCHRG_addr = 0x0
DISCHRG_regtype = 1
DISCHRG_noro = 0
FSD_addr = 0x140
FSD_regtype = 0
FSD_pulse_duration = 180

XXX_addr denote the modbus address of RIO where the UPS signal is connected
XXX_regtype is the type of modbus register (0:COIL, 1:INPUT_B, 2:INPUT_R, 3:HOLDING)
XXX_norois the configuration for NO/NC (normally open, normally closed contact)
FSD_pulse_duration defines the duration in ms of the inhibit pulse. if it's not defined signal has only one transition depending on noro configuration.

Regards
Dimitris.

@lgtm-com

This comment has been minimized.

@lgtm-com

This comment has been minimized.

@lgtm-com

This comment has been minimized.

@lgtm-com

This comment has been minimized.

@donnib
Copy link
Copy Markdown

donnib commented Jul 5, 2021

@dtsecon does this mean that i can setup my own Modbus TCP server and set NUT server to point to it and then i fill out data into the registries which NUT server would read , is that correct understood ?

@dtsecon
Copy link
Copy Markdown
Contributor Author

dtsecon commented Jul 5, 2021

@dtsecon does this mean that i can setup my own Modbus TCP server and set NUT server to point to it and then i fill out data into the registries which NUT server would read , is that correct understood ?

Yes, your assumption is correct although never tested. However, OL (online), OB (on battery), LB (low battery), HB (high battery), RB (replace battery) and FSD (forced shutdown) statuses are currently supported.

@dtsecon dtsecon closed this Jul 6, 2021
@dtsecon dtsecon reopened this Jul 6, 2021
@dtsecon
Copy link
Copy Markdown
Contributor Author

dtsecon commented Jul 6, 2021

@jimklimov hello,
I noticed that travis-ci is stuck and waiting for a status report although it has been setup and run a successful build for the PR repo. Is there something else needed to be done in order to proceed ?

Regards,
Dimitris.

@jimklimov
Copy link
Copy Markdown
Member

Sorry for the lag on my side. Travis CI has gone away from free FOSS builds, so a new dedicated NUT CI is being built. There are still some rough edges, but it should be usable by now and I'm bumping the existing PRs to build there. Thanks for waiting :\

@jimklimov
Copy link
Copy Markdown
Member

Hello, it seems that I may not update the PR source (your "master" branch) because it has github branch protection rules, probably: Couldn't update "master": Required status check "Travis CI - Branch" is expected.

Can you please disable that config point, seems Travis is not coming back...

@dtsecon
Copy link
Copy Markdown
Contributor Author

dtsecon commented Sep 14, 2021

Hello, it seems that I may not update the PR source (your "master" branch) because it has github branch protection rules, probably: Couldn't update "master": Required status check "Travis CI - Branch" is expected.

Can you please disable that config point, seems Travis is not coming back...

done!

@jimklimov
Copy link
Copy Markdown
Member

With CI finally somewhat rectified, it finds code issues in this PR - mostly due to platforms/standards mixup I think. A few that caught my eye in orange balls at https://ci.networkupstools.org/blue/organizations/jenkins/nut%2Fnut/detail/PR-1052/1 are:

[2021-09-15T05:45:20.637Z] In file included from generic_modbus.c:24:
[2021-09-15T05:45:20.637Z] generic_modbus.h:46:14: error: ISO C forbids forward references to 'enum' types [-Werror=pedantic]
[2021-09-15T05:45:20.637Z]    46 | typedef enum regtype regtype_t;
[2021-09-15T05:45:20.637Z]       |              ^~~~~~~

[2021-09-15T05:45:20.637Z] generic_modbus.h:55:14: error: ISO C forbids forward references to 'enum' types [-Werror=pedantic]
[2021-09-15T05:45:20.637Z]    55 | typedef enum devstate devstate_t;
[2021-09-15T05:45:20.637Z]       |              ^~~~~~~~
[2021-09-15T05:45:20.637Z] cc1: all warnings being treated as errors
  • Some uses of ulong - these should become an explicit-bitness type like uint32_t or uint64_t, or a size_t if some (time-related) library accepts that in the method signatures:
[2021-09-15T03:16:06.347Z] generic_modbus.c:437:14: error: expected ';' after expression
[2021-09-15T03:16:06.347Z]         ulong nsec = (end.tv_usec - start->tv_usec) / 1000000 + 1;
[2021-09-15T03:16:06.347Z]              ^
[2021-09-15T03:16:06.347Z]              ;

[2021-09-15T03:16:06.347Z] generic_modbus.c:437:9: error: use of undeclared identifier 'ulong'
[2021-09-15T03:16:06.347Z]         ulong nsec = (end.tv_usec - start->tv_usec) / 1000000 + 1;
[2021-09-15T03:16:06.347Z]         ^

[2021-09-15T03:16:06.347Z] generic_modbus.c:437:15: error: use of undeclared identifier 'nsec'
[2021-09-15T03:16:06.347Z]         ulong nsec = (end.tv_usec - start->tv_usec) / 1000000 + 1;
[2021-09-15T03:16:06.347Z]               ^

[2021-09-15T03:16:06.347Z] generic_modbus.c:438:34: error: use of undeclared identifier 'nsec'
[2021-09-15T03:16:06.347Z]         end.tv_usec -= 1000000 * nsec;
[2021-09-15T03:16:06.347Z]                                  ^

[2021-09-15T03:16:06.347Z] generic_modbus.c:439:23: error: use of undeclared identifier 'nsec'
[2021-09-15T03:16:06.347Z]         end.tv_sec += nsec;
[2021-09-15T03:16:06.347Z]                       ^

[2021-09-15T03:16:06.347Z] generic_modbus.c:442:14: error: expected ';' after expression
[2021-09-15T03:16:06.347Z]         ulong nsec = (start->tv_usec - end.tv_usec) / 1000000;
[2021-09-15T03:16:06.347Z]              ^
[2021-09-15T03:16:06.347Z]              ;

[2021-09-15T03:16:06.347Z] generic_modbus.c:442:9: error: use of undeclared identifier 'ulong'
[2021-09-15T03:16:06.347Z]         ulong nsec = (start->tv_usec - end.tv_usec) / 1000000;
[2021-09-15T03:16:06.347Z]         ^

[2021-09-15T03:16:06.347Z] generic_modbus.c:442:15: error: use of undeclared identifier 'nsec'
[2021-09-15T03:16:06.347Z]         ulong nsec = (start->tv_usec - end.tv_usec) / 1000000;
[2021-09-15T03:16:06.347Z]               ^
[2021-09-15T03:16:06.347Z] generic_modbus.c:443:34: error: use of undeclared identifier 'nsec'
[2021-09-15T03:16:06.347Z]         end.tv_usec += 1000000 * nsec;
[2021-09-15T03:16:06.347Z]                                  ^

[2021-09-15T03:16:06.347Z] generic_modbus.c:444:23: error: use of undeclared identifier 'nsec'
[2021-09-15T03:16:06.347Z]         end.tv_sec -= nsec;
[2021-09-15T03:16:06.347Z]                       ^
[2021-09-15T03:16:06.347Z] 10 errors generated.
[2021-09-15T03:16:06.347Z] *** Error code 1

@dtsecon
Copy link
Copy Markdown
Contributor Author

dtsecon commented Sep 16, 2021

With CI finally somewhat rectified, it finds code issues in this PR - mostly due to platforms/standards mixup I think. A few that caught my eye in orange balls at https://ci.networkupstools.org/blue/organizations/jenkins/nut%2Fnut/detail/PR-1052/1 are:

[2021-09-15T05:45:20.637Z] In file included from generic_modbus.c:24:
[2021-09-15T05:45:20.637Z] generic_modbus.h:46:14: error: ISO C forbids forward references to 'enum' types [-Werror=pedantic]
[2021-09-15T05:45:20.637Z]    46 | typedef enum regtype regtype_t;
[2021-09-15T05:45:20.637Z]       |              ^~~~~~~

[2021-09-15T05:45:20.637Z] generic_modbus.h:55:14: error: ISO C forbids forward references to 'enum' types [-Werror=pedantic]
[2021-09-15T05:45:20.637Z]    55 | typedef enum devstate devstate_t;
[2021-09-15T05:45:20.637Z]       |              ^~~~~~~~
[2021-09-15T05:45:20.637Z] cc1: all warnings being treated as errors
  • Some uses of ulong - these should become an explicit-bitness type like uint32_t or uint64_t, or a size_t if some (time-related) library accepts that in the method signatures:
[2021-09-15T03:16:06.347Z] generic_modbus.c:437:14: error: expected ';' after expression
[2021-09-15T03:16:06.347Z]         ulong nsec = (end.tv_usec - start->tv_usec) / 1000000 + 1;
[2021-09-15T03:16:06.347Z]              ^
[2021-09-15T03:16:06.347Z]              ;

[2021-09-15T03:16:06.347Z] generic_modbus.c:437:9: error: use of undeclared identifier 'ulong'
[2021-09-15T03:16:06.347Z]         ulong nsec = (end.tv_usec - start->tv_usec) / 1000000 + 1;
[2021-09-15T03:16:06.347Z]         ^

[2021-09-15T03:16:06.347Z] generic_modbus.c:437:15: error: use of undeclared identifier 'nsec'
[2021-09-15T03:16:06.347Z]         ulong nsec = (end.tv_usec - start->tv_usec) / 1000000 + 1;
[2021-09-15T03:16:06.347Z]               ^

[2021-09-15T03:16:06.347Z] generic_modbus.c:438:34: error: use of undeclared identifier 'nsec'
[2021-09-15T03:16:06.347Z]         end.tv_usec -= 1000000 * nsec;
[2021-09-15T03:16:06.347Z]                                  ^

[2021-09-15T03:16:06.347Z] generic_modbus.c:439:23: error: use of undeclared identifier 'nsec'
[2021-09-15T03:16:06.347Z]         end.tv_sec += nsec;
[2021-09-15T03:16:06.347Z]                       ^

[2021-09-15T03:16:06.347Z] generic_modbus.c:442:14: error: expected ';' after expression
[2021-09-15T03:16:06.347Z]         ulong nsec = (start->tv_usec - end.tv_usec) / 1000000;
[2021-09-15T03:16:06.347Z]              ^
[2021-09-15T03:16:06.347Z]              ;

[2021-09-15T03:16:06.347Z] generic_modbus.c:442:9: error: use of undeclared identifier 'ulong'
[2021-09-15T03:16:06.347Z]         ulong nsec = (start->tv_usec - end.tv_usec) / 1000000;
[2021-09-15T03:16:06.347Z]         ^

[2021-09-15T03:16:06.347Z] generic_modbus.c:442:15: error: use of undeclared identifier 'nsec'
[2021-09-15T03:16:06.347Z]         ulong nsec = (start->tv_usec - end.tv_usec) / 1000000;
[2021-09-15T03:16:06.347Z]               ^
[2021-09-15T03:16:06.347Z] generic_modbus.c:443:34: error: use of undeclared identifier 'nsec'
[2021-09-15T03:16:06.347Z]         end.tv_usec += 1000000 * nsec;
[2021-09-15T03:16:06.347Z]                                  ^

[2021-09-15T03:16:06.347Z] generic_modbus.c:444:23: error: use of undeclared identifier 'nsec'
[2021-09-15T03:16:06.347Z]         end.tv_sec -= nsec;
[2021-09-15T03:16:06.347Z]                       ^
[2021-09-15T03:16:06.347Z] 10 errors generated.
[2021-09-15T03:16:06.347Z] *** Error code 1

done!

@jimklimov jimklimov added the ready / gonna merge The PR is in final cycles leading to merge unless someone logs an objection before we hit the button label Sep 16, 2021
@jimklimov
Copy link
Copy Markdown
Member

Looks good to me and CI, hopefully tested with HW on your side at least? :)

One question about the side fixes for asciidoc builds -- did your tools complain about the lack of equals sign? Seems those versions on CI farm are okay with it either way, at least they do not fail due to CLI :)
Did you verify that the results of those docs builds are equivalent?

@dtsecon
Copy link
Copy Markdown
Contributor Author

dtsecon commented Sep 17, 2021

Looks good to me and CI, hopefully tested with HW on your side at least? :)

One question about the side fixes for asciidoc builds -- did your tools complain about the lack of equals sign? Seems those versions on CI farm are okay with it either way, at least they do not fail due to CLI :)
Did you verify that the results of those docs builds are equivalent?

the driver supports any UPS with relay contact signals via a remote I/O device (with RS485 or TCP/IP interface). It has been thoroughly tested against PULS UB40.241 but is expected to work with any other similar UPS.

there is a missing '=' in a2x --xsltproc-opts switch argument, specifically in conficure.ac and docs/man/Makefile.am. It seems that "=" is mandatory for a2x to run successfully otherwise building procedure (with --with-doc(s)) couldn't compile man pages at all.

@jimklimov
Copy link
Copy Markdown
Member

jimklimov commented Sep 19, 2021 via email

@dtsecon
Copy link
Copy Markdown
Contributor Author

dtsecon commented Sep 20, 2021

Thanks for clarifications. Just to know: what version of a2x and on what OS did not work for you with the older Makefiles? Because that part did not change for a few years AFAIK and worked on many different platforms. Wondering if yours is too new or too old? Or some fork? Either way, new CLI syntax seems to work for CI builders too, so thanks!

On Fri, Sep 17, 2021, 10:34 dtsecon @.***> wrote: Looks good to me and CI, hopefully tested with HW on your side at least? :) One question about the side fixes for asciidoc builds -- did your tools complain about the lack of equals sign? Seems those versions on CI farm are okay with it either way, at least they do not fail due to CLI :) Did you verify that the results of those docs builds are equivalent? the driver supports any UPS with relay contact signals via a remote I/O device (with RS485 or TCP/IP interface). It has been thoroughly tested against PULS UB40.241 but is expected to work with any other similar UPS. there is a missing '=' in a2x --xsltproc-opts switch argument, specifically in conficure.ac and docs/man/Makefile.am. It seems that "=" is mandatory for a2x to run successfully otherwise building procedure (with --with-doc(s)) couldn't compile man pages at all. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1052 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMPTFF2S733G24U6IXLJFTUCL4QBANCNFSM47IFNFEA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

Here is the platform used for building:

OS: Ubuntu 20.04, a2x 9.0.0rc1 (provided by asciidoc-base_9.0.0~rc1-1), GNU Make 4.2.1.

@jimklimov jimklimov merged commit 8d0cc97 into networkupstools:master Sep 20, 2021
@jimklimov
Copy link
Copy Markdown
Member

Still looks good, merging! Thanks for the driver and for a2x fixes to move forward :)

@jimklimov
Copy link
Copy Markdown
Member

Cheers, one more question as I'm going over the code to address some style and stricter compiler warnings: in many places you use arch-dependent "int" and "uint", shouldn't there be more definitively-sized types (uint16_t and so on) to correspond to some wire protocol and library structures?

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

Labels

modbus ready / gonna merge The PR is in final cycles leading to merge unless someone logs an objection before we hit the button

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants