search

epics-modules / opcua

EPICS Device Support for OPC UA
Other
19 stars 13 forks source link

Element records set Item ConnectionStatus::up too early #161

Closed dirk-zimoch closed 1 month ago

dirk-zimoch commented 3 months ago

manageStateAndBiniProcessing in devOpcua.cpp changes the Item state from ConnectionStatus::initialRead to ConnectionStatus::up. If the record is an element of a structured Item, then the first element processing already sets the whole Item state to up and other elements processing later will see up instead of initialRead and may not initialize the record correctly.

I noticed this while working on the enum improvement: Enum elements bound to mbbi/o records do not initialize their string tables when other elements had been initialized before.

ConnectionStatus should be a property of the RecordConnector (or DataElement) but not of the Item. Instead of RecordConnector::setState() propagating the state to the Item (possibly multiple elements to the same Item), Item->setState() should propagate the state to all RecordConnectors (or DataElements). Maybe this can be combined with setIncomingEvent() which traverses the element tree anyway and is often called together with setState().

ralphlange commented 3 months ago

Agree. While the actual connection state is obviously a property of the connection, holding a copy at the intermediate (item) layer is wrong. Each leaf (of the data structure) data element needs a copy that it can handle independently, related to the processing of its record. That's a leftover from non-structured times when the items were the leaves.

As mentioned, DataElement is the best location for that connection state copy. (On the OPC UA side of things; it doesn't really belong to the record connection.)

Also agree that events that originate at the item level (like sending out the initial readback) need to propagate to all data elements.

dirk-zimoch commented 3 months ago

I think the ConnectionStatus should be in the RecordConnector rather than in the DataElement because the itemRecord is connected to the root of the element tree and setting its ConnectionStatus would traverse to all element records again. Each record shall only set its own ConnectionStatus but each Item shall set the ConnectionStatus of all records in the whole element tree.

dirk-zimoch commented 3 months ago

Fixed (I think). PR tomorrow (or later).