Restructure C++ Classes in order to reflect SilecsClasses

• each PLC has silecsClasses
• each Device holds a link to it's silecsClass
• each Device has Blocks
• each Block has Registers It should be possible to run recv/send for all devices of a class at once (per Block) --> static method at device-level ?

In old impl. Block was not device-specific !! Thats a huge Bug !! ( SilecsPLC.cpp:533 of v0.10.0 )

First re-evaluate if the suggested architecture is correct/ makes sense.

changed milestone to %3.0.0

Has overlap with #17 (closed)

changed time estimate to 1w

Here is my suggestion of the hierarchy:

1. SilecsService - stays as the topmost - changing it to either a singleton or a fully static class. Currently it's not one nor the other. :)
2. Multiple PLCs are held by the SilecsService. Each PLC can take multiple parameter files (in turn SILECS/FESA classes - currently named Cluster). Each of those represents a class running on the PLC. The "parameter file" (or better yet the contents of it) would be represented by another C++ class called either SilecsDesign/SilecsClass/SilecsInstance (not sure what the best name would be). So.. each PLC owns multiple SilecsDesign/Class/Instances.
3. Each SilecsDesign/Class/Instance owns Devices, which in turn own Blocks which own Registers. (unlike currently where PLC owns all the Devices and all the Blocks, while the Device owns all the Registers)
4. With this modification we would have a clear hierarchy of classes, which should also cleanup the current design where "all the" classes are friends of each other, and pointers are passed around everywhere.

Thank you for the suggestions!

1. SilecsService - stays as the topmost - changing it to either a singleton or a fully static class. Currently it's not one nor the other.

Agreed, let's have it a fully static class. Using a singleton can be annoying for unit-testing (in case we add c++ unit testing one day)

2. Multiple PLCs are held by the SilecsService

Fine for me.

2. Each PLC can take multiple parameter files (in turn SILECS/FESA classes - currently named Cluster). Each of those represents a class running on the PLC. The "parameter file" (or better yet the contents of it) would be represented by another C++ class called either SilecsDesign/SilecsClass/SilecsInstance (not sure what the best name would be). So.. each PLC owns multiple SilecsDesign/Class/Instances.

Discussion required on this one:

Parameter files are specific per PLC and per SilecsDeploy, not per SilecsDesign. One param file can reference multiple SilecsDesigns. (E.g. in ValveGaugeDU).

So I would say, yes, each PLC can have multiple parameter files (That might be useful for clients, talking to multiple classes running on the same PLC). However, these cannot be represented by a SilecsDesign C++ class.

Think I would design that part the following:

• The param file is read out, and a C++ 'SilecsDeploy' is created for each.
• Each C++ 'SilecsDeploy' owns 1..n SilecsDesign instances, as well created according to the param file.

You think that could work ?

3. Each SilecsDesign/Class/Instance owns Devices, which in turn own Blocks which own Registers. (unlike currently where PLC owns all the Devices and all the Blocks, while the Device owns all the Registers)

Agreed that each SilecsDesign should own Devices. Not sure if the Blocks and Registers should be owned by the Devices or maybe owned by the PLC and only references by the Devices. I suppose we need to keep the possibility to get a specific Block for all Devices, and to get all Blocks of a specific Device. Getting a Block of for all Devices might need extra-PLC requests when we have to iterate on all devices using that block. What do you think ?

Parameter files are specific per PLC and per SilecsDeploy, not per SilecsDesign. One param file can reference multiple SilecsDesigns. (E.g. in ValveGaugeDU).

So I would say, yes, each PLC can have multiple parameter files (That might be useful for clients, talking to multiple classes running on the same PLC). However, these cannot be represented by a SilecsDesign C++ class.

Think I would design that part the following:

• The param file is read out, and a C++ 'SilecsDeploy' is created for each.
• Each C++ 'SilecsDeploy' owns 1..n SilecsDesign instances, as well created according to the param file.

You think that could work ?

Alright so we found out that SilecsDesign is not a good name for that class :) In reality the current Cluster class doesn't hold anything special, so I don't see a need for both a SilecsDesign and a SilecsDeploy class. The library also probably shouldn't care how FESA part is structured. So I would suggest that a single class is used which holds Devices. For example SilecsInstance (which represents one parameter file). Would that make sense?

Agreed that each SilecsDesign should own Devices. Not sure if the Blocks and Registers should be owned by the Devices or maybe owned by the PLC and only references by the Devices. I suppose we need to keep the possibility to get a specific Block for all Devices, and to get all Blocks of a specific Device. Getting a Block of for all Devices might need extra-PLC requests when we have to iterate on all devices using that block. What do you think ?

I did think about how to address this, but forgot to write about it in the issue. Regarding getting specific block for all devices and all blocks of a specific device:

• For device mode the proposed architecture make sense, since a single DB contains everything for that particular device. So everything can be implemented within the Device/Block classes. In addition the PLC can have a get block for all devices function, which simply iterates over all the devices. Which is already the minimal number of requests.
• For block mode for getting blocks of a single device everything still makes sense. It's all handled by the Device/Block classes. The only inconsistency is then getting a block for all devices in block mode. This case with the proposed architecture would require more requests if done by looping through each Device. So for this specific case the PLC should have a get block for all devices function (same as for device-mode) however it wouldn't simply loop though all devices, but would instead gather the required addresses, offsets, DBs numbers etc. and do a single request to the PLC. And then distribute the received data to the underlying Devices/Blocks/Registers.

This was my idea. If you prefer to have the PLC own the blocks. We can also leave it that way and have the devices reference them (either way both ways have some advantages and some disadvantages). Effort wise it's very similar I would say.

For example SilecsInstance (which represents one parameter file). Would that make sense?

Nah, if you want some C++ construct which represents a parameter File, than call it e.g SilecsParameterConfig or similar. SilecsInstance has the risk to be confused with a FESA device instance.

The library also probably shouldn't care how FESA part is structured.

Yes and no ... you are right, ideally Silecs should be control-system agnostic. However, currently we are rather tightly entangled with FESA ... we have SilecsDeploy And SilecsDesign which mirror FESA structures. I would say, as long as the silecs codegen has these structures, it is fine to as well have them in the lib, if required. (If you don't need them, as well fine for me … though if you feel that they would help, I think we should reuse the naming from the codegen)

I did think about how to address this, but forgot to write about it in the issue. Regarding getting specific block for all devices and all blocks of a specific device:

• For device mode the proposed architecture make sense, since a single DB contains everything for that particular device. So everything can be implemented within the Device/Block classes. In addition the PLC can have a get block for all devices function, which simply iterates over all the devices. Which is already the minimal number of requests.
• For block mode for getting blocks of a single device everything still makes sense. It's all handled by the Device/Block classes. The only inconsistency is then getting a block for all devices in block mode. This case with the proposed architecture would require more requests if done by looping through each Device. So for this specific case the PLC should have a get block for all devices function (same as for device-mode) however it wouldn't simply loop though all devices, but would instead gather the required addresses, offsets, DBs numbers etc. and do a single request to the PLC. And then distribute the received data to the underlying Devices/Blocks/Registers.

This was my idea. If you prefer to have the PLC own the blocks. We can also leave it that way and have the devices reference them (either way both ways have some advantages and some disadvantages). Effort wise it's very similar I would say.

I don't have a strong preference on that one. Your approach sound ok to me ... let's go for it!

mentioned in issue #25 (closed)

mentioned in merge request !54 (merged)

mentioned in merge request !55 (merged)

Fixed by above merge requests

closed