robotology / blockfactory

A tiny framework to wrap algorithms for dataflow programming
https://robotology.github.io/blockfactory
GNU Lesser General Public License v2.1
40 stars 16 forks source link

Add a unique identifier to Block objects #16

Closed diegoferigo closed 5 years ago

diegoferigo commented 5 years ago

Probably coming from the Simulink inheritage and its Simstruct, we never had the need to assign to a block a unique id or name. However, especially with the new plugin architecture, it might be worth adding the following to blockfactory::core::Block

protected:
    std::string name;
public:
    Block(const std::string& name);
    std::string getName() const;
    void setName() const;

Since these are not virtual methods, the downstream classes that implement the interface would not need to be modified.

For what concern the S-Function, we should store it as additional RTW parameter and modify the TLC file to get this information. If we ever want in the future to create a system to override mask parameters, being able to access the block object with a unique id will be fundamental.

It is not clear to me if we should also add a getName method in the BlockInformation interface.

traversaro commented 5 years ago

With "unique" you mean: unique to each instance of the block in a given simulation? Or in the given model/diagram? Who should assign this names, the engine?

Some possible targets (such as Drake) already have a concept of unique instance name, but it is already managed at the level of the engine (see https://drake.mit.edu/doxygen_cxx/classdrake_1_1systems_1_1_system_base.html#ad5260b9627048b854b45d05ed34adc22). If this name is completed managed by the engine, perhaps it make sense to just have a getName(), and no non-virtual setName() ?

diegoferigo commented 5 years ago

Talking about Simulink, I was thinking that the S-Function read the name of the block from the model and assigns it to the block object. In order to be unique for each block, it should be the absolute name from the root of the model (if you imagine having subsystems and seeing it as a graph).

Another option in order to minimize (but not remove) name collisions, is to use <parent_scope>/<block_name>. This would simplify accessing a block but name collisions should be checked. Think of an example for this use case. If you want to override a mask parameter of a single block instance from the autogenerated code, you can create a conf file (xml?) that store the information to override the value stored in the Simulink mask (that is hardcoded in the sources). This substitution logic must address the block somehow, and using a shorter name id rather than the absolute location in the model can be more robust (for example to changes in the parent's scope structure). RIght now we only have the information of the C++ class associated to the block, and in most cases there are more than one block for each type.

If this name is completed managed by the engine, perhaps it make sense to just have a getName(), and no non-virtual setName() ?

Another option I like is having in core::Block the following:

std::string getName(core::BlockInformation*) const;

And putting the logic of gathering the name inside the core::BlockInformation class since it is engine-related.

traversaro commented 5 years ago

I was thinking that the S-Function read the name of the block from the model and assigns it to the block object.

How? I found ssGetModelName, but I cannot find an equivalent "set" method. Sorry, I got what you mean. Yes, I suspect that getting it through BlockInformation is more coherent with the overall infrastructure.

DanielePucci commented 5 years ago

@diegoferigo

Probably coming from the Simulink inheritage and its Simstruct, we never had the need to assign to a block a unique id or name.

Back in time, we used the block priority as a unique identifier from which we could define block execution order