Improvements and cleanup of signals and ports

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

Improvements and cleanup of signals and ports #33

Closed diegoferigo closed 5 years ago

diegoferigo commented 5 years ago

Since the time of wb-toolbox there was some confusion between properties and information assigned to signals and ports. Sometimes things related to one were used on the other.

This PR:

Introduces a new core::Port class that collects most of the information about ports, and removes it from core::Signal and core::BlockInformation.
Removes the size optional argument when asking a Signal object from BlockInformation implementations. This should never be the case since this information should be stored inside the BlockInformation object in the configuration phase and the blocks should not have the power of asking a signal with a size that might mismatch with the right one.
Cleans buffer initialization of the core::Signal class. Now the size of the buffer is specified when the buffer pointer is stored. This should provide more robust APIs since there won't be anymore the need to remember to call Signal::setWidth in order to have a valid object.

diegoferigo commented 5 years ago

One thing to note (even if not directly related to this PR) is that there is no documentation on where to install the blockfactory plugins in the install prefix.

I tried to handle it with the install_blockfactory_plugin CMake function:

https://github.com/robotology/blockfactory/blob/39b3d4441957b90e0d2c71e8bde2fe47d0e1b055/cmake/BlockFactoryPlugin.cmake#L93-L100

And there is a message displayed when configuring the project (in addition to the website):

https://github.com/robotology/blockfactory/blob/39b3d4441957b90e0d2c71e8bde2fe47d0e1b055/cmake/BlockFactoryPlugin.cmake#L85-L91

Additionally, w.r.t. to debian packaging, it would be good if the default installation prefix was in the search path even without the need to define BLOCKFACTORY_PLUGIN_PATH, so that if in the future we have deb package of BlockFactory + WB-Toolbox, they will just work out of the box without the need to define any env variable.

This is more complicated. If we install plugins in one of the default linker folder, we get the same problems due to LD_LIBRARY_PATH we recently discussed in https://github.com/robotology/ycm/issues/199 and https://github.com/robotology/robotology-superbuild/issues/97.

diegoferigo commented 5 years ago

@traversaro I would keep this discussion going in a new issue, feel free to open it. In the meantime, I proceed merging this PR.

traversaro commented 5 years ago

I tried to handle it with the install_blockfactory_plugin CMake function:

You are right, sorry for missing it!

traversaro commented 5 years ago

I would keep this discussion going in a new issue, feel free to open it. In the meantime, I proceed merging this PR.

Ok, new issue open in https://github.com/robotology/blockfactory/issues/34 .

diegoferigo commented 5 years ago

Merged, thanks!