I actually don't like this. If an invalid value is found, I'd rather that a protocol violation or weird gets generated instead since it actually indicates a protocol error (I believe it does in this case at least).
What do you think about that?
.Seth
Added another parameter to modbus events that carry register arrays to
the script-layer which indicates the associated byte count from the
message (allowing for invalid values to be detected):I actually don't like this. If an invalid value is found, I'd rather that a protocol violation or weird gets generated instead since it actually indicates a protocol error (I believe it does in this case at least).
I can add a protocol violation and not generate the event in these cases… but do you care whether the extra parameter is provided to the events in the valid cases (I don't think it's hurting anything) ?
And really I'm just trying fix stuff that results in a crash... there's also a bunch of &check attributes in modbus-protocol.pac that seem to indicate protocol violations, but they don't do anything since that attribute is a no-op in binpac. I didn't intend to go through and fix all those, but I suppose I could if you're concerned about that kind of thing.
Jon
I can add a protocol violation and not generate the event in these cases… but do you care whether the extra parameter is provided to the events in the valid cases (I don't think it's hurting anything) ?
You're right, it's not hurting anything, but I think the events are redundant if they include those lengths since it's implicitly included in the number of registers included. As I've changed events and added new ones, I try not include length fields that are implicit in other fields. This case is a little weird because the protocol itself is a little weird, but I still think it makes sense to leave the length out.
And really I'm just trying fix stuff that results in a crash... there's also a bunch of &check attributes in modbus-protocol.pac that seem to indicate protocol violations, but they don't do anything since that attribute is a no-op in binpac.
Yeah, I left those there as place holders so that we didn't need to refer back to the standard for those values when we port this to binpac++. Feel free to ignore those.
.Seth