vapor-ware / synse-modbus-ip-plugin

Modbus over TCP/IP plugin for Synse
GNU General Public License v3.0
2 stars 3 forks source link

Modbus bulk read #58

Closed MatthewHink closed 4 years ago

MatthewHink commented 4 years ago

Something is odd with the modbus plugin. We're not reading values where we should be.

Synse read:

ubuntu@w1:~$ export SYNSE_IP=10.42.4.226
ubuntu@w1:~$ curl http://${SYNSE_IP}:5000/v3/read/05a3062d-bc77-5005-a00e-b67e11af005f
[

]

Protocol Client direct read:

ubuntu@w1:~$ sudo -HE env PATH=$PATH ./pcli mip 10.193.9.244:502 get coil 33
DEBU[0000] readCoil 0x0033, decimal 51
INFO[0000] false
MatthewHink commented 4 years ago

Sample test failure for the above case:

[2020-07-27 11:29:15,483] site_tests:184 (ERROR) - --- FAIL: Read pCOWeb Compressor 4 VFD Running ---
[2020-07-27 11:29:15,483] site_tests:186 (ERROR) - scan_device: {'id': '05a3062d-bc77-5005-a00e-b67e11af005f', 'alias': '', 'info': 'Compressor 4 VFD Running', 'type': 'switch', 'plugin': '59aa69eb-a5da-569f-aaa4-3ebbdb72530b', 'tags': ['system/type:switch'], 'metadata': {'model': 'pCOWeb'}}
[2020-07-27 11:29:15,483] site_tests:190 (ERROR) - data: ['reading length < 1']
MatthewHink commented 4 years ago

Looking at this, the coil readings are definitely messed up. There is not enough info in the logs to debug. Bulk read coil tests are commented out.

This code got changed around substantially since Synse v2, so this will likely take awhile.

Going to start with writing functional tests.

MatthewHink commented 4 years ago

coverage.html.zip

^ Major coverage gaps in coils / holding registers / input registers.

MatthewHink commented 4 years ago

Making some progress:

Changing all coil handlers to use "coil" gets correct results however we are reading them one at a time rather than all at once (103 round trips versus 1).

Using "read_only_coil" on all coils but 3 (BMS Start on the VEM) reads only coil 3 and no others.

Still digging.

MatthewHink commented 4 years ago

All of the underlying data structures / functions have been changed and / or renamed here since I got this working for Synse v2, so this will take a bit to sort out what it's doing.

MatthewHink commented 4 years ago

Core issue here is that the batched reads to reduce network round trips is broken. Every coil / register read is a single round trip.