Malformed autogenerated code if the scoped name of the block contains newline character

[diegoferigo]

Related to #16, on complex model with many subsystem, the scoped name of the block can be very long. During the autogeneration process, Simulink Coder tries to break lines properly, but it seems failing in this case. The obtained code is something similar:

// Store the block name
        blockInfo->setUniqueBlockName("torqueBalancingYoga/Dynamics and Kinematics/Kinemati
cs/DotJ Nu l_sole/S-Function");

In C++ is legit breaking a line, but only if a "" pair is added in between. The outcome of Simulink Coder is a source file that does not compile.

I already tried to apply a style with clang-format, but the malformed code does not get fixed anyway.

[traversaro]

Just to understand, is this a Coder bug?

[diegoferigo]

From a point of view, yes. The problem source is the following line in the TLC file:

https://github.com/robotology/blockfactory/blob/7b302babd47b73be6871771736aed1a80bee3dac/matlab/BlockFactory.tlc#L179-L180

%<blockUniqueName> is a variable that is substituted with the a long string, and since the line get too long, the coder breaks it in an abrupt way.

[traversaro]

Would it make sense to report it to MathWorks? Perhaps they can suggest a workaround.

[diegoferigo]

I investigated a bit more on this. It is not a Simulink nor a Simulink Coder problem.

The block name in the Simulink model was going to a new line and was adding few spaces in the new line. This \n (I guess) is not visible from the model, but the Simulink APIs (ssGetPath) reads everything. When the coders substitutes the variable in the TLC with this string, it interprets the new line character and generates a malformed source. It took a while to understand it :sweat_smile:

cc @gabrielenava please keep this in mind from now on

[traversaro]

Cool. I guess it is too late for WB-Toolbox blocks, but I wonder if in general choosing block names without spaces or in general any whitespace may be a convenient choice.

[diegoferigo]

The thing is that we cannot rely on it. Users can change those names and often they are descriptive of how they are used. Forcing users to avoid spaces is a bit stretched in my opinion.

If we will have problems with them, I think the best option is substituting spaces with e.g. _ when they are serialized into the autogenerated source. The only drawback of doing so is the hypothetical access to mask parameters (that is just an idea today). In this case the user needs to know which is the scoped name of the block, but we can think of enhancing the GeneratedCodeWrapper.h to provide information about the model (for instance printing all the blocks) by inspecting it.

p.s. Also, we could check if a \n is present in the string and raise an error. All cool things that we can implement in the future to improve robustness.

[diegoferigo]

I guess it is too late for WB-Toolbox blocks

Btw, if we want to do it, this is the right moment. Since we have to run a script in any case to update to the new version (https://github.com/robotology/wb-toolbox/issues/167), it would not be a problem to also substitute the names. It might be worth to take advantage of it now.

[traversaro]

Forcing users to avoid spaces is a bit stretched in my opinion.

I was not thinking about forcing users to avoid spaces, just as our internal guidelines to avoid them (similary to how we avoid spaces in directory names). However, if we do that then probably we will not catch if we have spaces-related bugs, so it is probably better not to do that.

[diegoferigo]

In my opinion, we can change the current block names of the library and remove the spaces. But in any case we should support spaces since users are free to change the name.

Another point of failure in my opinion is that / is an allowed character in the block name. The problem is that when you get instead the scoped block name, the different levels are separated by, guess what, the / character. This means that we cannot exploit this information to figure out the hierarchy of the block in the model.

[diegoferigo]

Running the provided script will automatically remove the spaces from Simulink Models. However, if the user has modified the default name of a block, the extra spaces won't be removed. E.g. Set References Robot1 it will become SetReferences Robot1.

[diegoferigo]

cc @aikolina