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

switched to modbus_new_tcp_pi #50

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitmodules
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
[submodule "libmodbus"]
path = libmodbus
url=https://github.com/v-zhuravlev/libmodbus
branch = v3.1.4
branch = myssta-patch
4 changes: 1 addition & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,7 @@ where:

for Modbus Encapsulated (RTU over TCP):
IPv4 of Modbus gate, for example: `enc://192.168.1.1`
TCP port may also be redefined (from Modbus default 502) if needed: `enc://192.168.1.1:5000`

Note: DNS names are not supported for TCP and RTU over TCP
TCP port may also be redefined (from Modbus default 502) if needed: `enc://192.168.1.1:5000`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Trailing spaces. :)


for Modbus RTU over serial:
Serial connection parameters in a form of:
Expand Down
17 changes: 13 additions & 4 deletions src/modbus.c
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,12 @@ int zbx_modbus_read_registers(AGENT_REQUEST *request, AGENT_RESULT *result)
modbus_free(ctx);
return SYSINFO_RET_FAIL;
}
modbus_set_slave(ctx, slave_id);
if (modbus_set_slave(ctx, slave_id) == -1)
{
SET_MSG_RESULT(result, strdup("Check slaveid parameter"));
modbus_free(ctx);
return SYSINFO_RET_FAIL;
}

//<reg> set register to start from
errno = 0;
Expand Down Expand Up @@ -449,30 +454,34 @@ void create_modbus_context(char *con_string, modbus_t **ctx_out, int *lock_requi

char host[100];
int port = MODBUS_TCP_DEFAULT_PORT;
char port_str[10];
Copy link
Collaborator

Choose a reason for hiding this comment

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

sprintf()ing MAX_INT can overflow this buffer.


if (strncmp(con_string, "enc://", strlen("enc://")) == 0)
{
*lock_required_out = 1;
con_string += strlen("enc://");
sscanf(con_string, "%99[^:]:%99d[^\n]", host, &port);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why don't we just read port as %s?

sprintf(port_str, "%d", port);
*lock_key = hash(host) % NSEMS;
*ctx_out = modbus_new_rtutcp(host, port);
*ctx_out = modbus_new_rtutcp_pi(host, port_str);
}
else if (strncmp(con_string, "tcp://", strlen("tcp://")) == 0)
{
*lock_required_out = 0;
con_string += strlen("tcp://");
sscanf(con_string, "%99[^:]:%99d[^\n]", host, &port);
sprintf(port_str, "%d", port);
*lock_key = hash(host) % NSEMS;
*ctx_out = modbus_new_tcp(host, port);
*ctx_out = modbus_new_tcp_pi(host, port_str);
}
else
{ // try Modbus TCP

*lock_required_out = 0;
sscanf(con_string, "%99[^:]:%99d[^\n]", host, &port);
sprintf(port_str, "%d", port);
*lock_key = hash(host) % NSEMS;
*ctx_out = modbus_new_tcp(host, port);
*ctx_out = modbus_new_tcp_pi(host, port_str);
}
}

Expand Down
12 changes: 12 additions & 0 deletions tests/test_00modbus_connection_string.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ class TestModbusConnectionString(object):

host = "172.16.238.2:5020"
host_rtutcp = "172.16.238.2:5021"
host_dns = "modbus-server:5020"
host_rtutcp_dns = "modbus-server:5021"

# test enc:// (rtu over tcp)

Expand All @@ -18,6 +20,16 @@ def test_modbus_tcp(self):
key = "modbus_read[tcp://"+self.host+",1,1,3,uint16]"
assert zabbix_get(key) == '49677'

# test enc:// (rtu over tcp) using hostname
def test_modbus_test_rtutcp_dns(self):
key = "modbus_read[enc://"+self.host_rtutcp_dns+",1,0,3,uint16]"
assert zabbix_get(key) == '49807'

# test tcp:// (plain tcp) using hostname
def test_modbus_tcp_dns(self):
key = "modbus_read[tcp://"+self.host_dns+",1,1,3,uint16]"
assert zabbix_get(key) == '49677'

# test serial /dev/ttyS0
@pytest.mark.skip("implement this first")
def test_modbus_serial(self, host):
Expand Down
6 changes: 3 additions & 3 deletions tests/test_01modbus_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def test_no_IP(self, host):
def test_bad_IP(self, host):
"""Test bad IP"""
key = "modbus_read_registers[badIP,3,14,3,uint32,BE,0]"
assert zabbix_get(key) == 'ZBX_NOTSUPPORTED: Network is unreachable'
assert zabbix_get(key) == 'ZBX_NOTSUPPORTED: Connection refused'

def test_no_slaveID(self, host):
key = "modbus_read_registers[{HOST.CONN},,14,3,uint32,BE,0]"
Expand Down Expand Up @@ -63,7 +63,7 @@ def test_bad_register_string(self, host):

def test_bad_slaveid_integer(self, host):
key = "modbus_read_registers["+host+",5000,99,3,uint32,BE,0]"
assert zabbix_get(key) == 'ZBX_NOTSUPPORTED: Illegal data address'
assert zabbix_get(key) == 'ZBX_NOTSUPPORTED: Check slaveid parameter'
def test_bad_slaveid_string(self, host):
key = "modbus_read_registers["+host+",bad,1,3,uint32,BE,0]"
assert zabbix_get(key) == 'ZBX_NOTSUPPORTED: Check slaveid parameter'
assert zabbix_get(key) == 'ZBX_NOTSUPPORTED: Check slaveid parameter'