[development] glue_generator segfaults for %[IQ]X???.Z (Z>=8)

[innir]

The glue generator assumes that coil and discrete input registers of external Modbus devices are of the form %[IQ]X???.Z with ??? >= 100 and 0 <= Z < 8 which is not necessarily true. I'm using a Weidmüller Modbus field bus coupler UR20-FBC-MOD-TCP-V2 with 48 DIs and 32 DOs and they are mapped as %IX100.[0-31] and %QX100.[0-47].

[garretfick]

This is a bit of a problem that probably needs some discussion. We should fix the glue generator to not segfault. However, even if we do that, you would not be able to access the values because Z < 7 is assumed throughout.

Can you give some more details about the modbus mapping? It is on my roadmap to enable more flexible modbus register mapping, and this input would be helpful in designing these capabilities.

[innir]

The manual of the fieldbus coupler can be downloaded here: https://download.weidmueller.com/asset/download/file//49497 (pages 63 - 78). It might be related to the 'feature' they call Packed process data. Used DI modules are UR20-16DI-P (p. 160ff), DO modules are UR20-16DO-P (p. 228ff).

This mbconfig.cfg works with the master branch:

Num_Devices = "1"
Polling_Period = "100"
Timeout = "1000"
# ------------
#   DEVICE 0
# ------------
device0.name = "UR20"
device0.slave_id = "0"
device0.protocol = "TCP"
device0.address = "192.168.78.109"
device0.IP_Port = "502"
device0.RTU_Baud_Rate = "115200"
device0.RTU_Parity = "None"
device0.RTU_Data_Bits = "8"
device0.RTU_Stop_Bits = "1"

device0.Discrete_Inputs_Start = "0"
device0.Discrete_Inputs_Size = "48"
device0.Coils_Start = "32768"
device0.Coils_Size = "32"
device0.Input_Registers_Start = "0"
device0.Input_Registers_Size = "0"
device0.Holding_Registers_Read_Start = "0"
device0.Holding_Registers_Read_Size = "0"
device0.Holding_Registers_Start = "0"
device0.Holding_Registers_Size = "0"

[garretfick]

Thanks. I'll have a look at the manual over the next few days.

Are you sure that the the master implementation actually correctly communicates all of the DI/coil values? My understanding, which might be wrong, would be that only some of the values would actually be transmitted.

[garretfick]

@innir I've had a look at the spec. From my understanding, what you need is the ability to access any of the DI or coils, but not specifically with the sub-indices 0-31 or 0-47. Is this understanding correct?

I've also looked at the implementation in master, and concluded that the mapping might have partially worked by chance, but not by design.

[innir]

@garretfick Correct, I don't mind how they are mapped, I just wand to be able to access them all.

[garretfick]

@innir This is on my roadmap as I have similar issues with the modbus master mapping. However, my priority right now is focused on driving towards a release.

If you are up for it, I can provide detailed guidance on how I think this this should work and should be integrated it the runtime. Otherwise, I probably won't get to this for a few months.

[innir]

@garretfick I can't promise anything but I would try if you could provide some hints etc.

[garretfick]

I'll start with my definition of the problem. Hopefully @thiagoralves can chime in with some additional thoughts.

Problem

The current modbus master implementation (in both the master and development branches) use a simple to understand approach for mapping located variables to DI, coils, registers and holding registers. This approach is based on the data type of the located variable. With bit manipulation functions in the structured text implementation, you can achieve most representations.

However, this does have some drawbacks.

1. Data types that span multiple locations (such as a 32-bit integer) are difficult to map to located variables
2. Some modbus slaves (such as SunSpec Modbus) use holding registers for all values and will contain a mixture of writable and non-writable values. Further, the data type of the located variable does not imply the a coil, holding register, etc.

We desire a flexible mechanism to map located variables to the modbus master that will support these use cases.

[innir]

@garretfick I turns out, that we had some misinterpretation of the documentation - so our issue is fixed :-)

The reason why it worked with master as we did is, that c++ basically stores bool_input[1024][8] in a linearly allocatable memory block, so bool_input[100][8] == bool_input[101][0]. So we just rewrote our mapping ...

I'll have a look at glue_generator in development anyway to fix the segfault at least.

[garretfick]

You hit on it exactly and why I was asking if things actually worked. My guess is that it compiled but didn't behave as you expected.

[thiagoralves]

That's correct. Gluevariables on master expects the bool vectors to have the format [X][Z] where 0 <= Z <= 7. For any Z > 7 you will have an undefined behavior. On the master, since it is just a simple pointer map, it works because for the compiler the memory is flat anyway. The dev branch has a much more sophisticated approach, and that's why it doesn't work. @garretfick should we include a check on gluegenerator for invalid locations (i.e. %IX0.9)? Then it should just stop the compilation process with an error message alerting the user about the invalid location.

[garretfick]

@thiagoralves we (or I) should definitely write a check.

One option would be to "auto-fix" designs to map to prior indices. I'm inclined to not implement that because the behavior was previously undefined and possibly buggy. Any thoughts?

[garretfick]

https://github.com/thiagoralves/OpenPLC_v3/pull/114 is intended to address this. I haven't tested if a complete build fails as I don't have an environment on this computer that can test if the cmake build fails if a step returns non-zero.

[innir]

@garretfick Thanks for the fix - I added two comments. cmake will fail if a command returns a non-zero value.