iCubBlinker compilation failure against YARP-3.6

[traversaro]

Since https://github.com/robotology/yarp/pull/2780 has been merged, funny-things is failing with this error:

2021-12-22T16:04:29.7621615Z -- Build files have been written to: /__w/robotology-superbuild/robotology-superbuild/build/src/funny-things
2021-12-22T16:04:37.1402008Z [235/248] Performing build step for 'funny-things'
2021-12-22T16:04:37.1403659Z FAILED: src/funny-things/CMakeFiles/YCMStamp/funny-things-build /__w/robotology-superbuild/robotology-superbuild/build/src/funny-things/CMakeFiles/YCMStamp/funny-things-build 
2021-12-22T16:04:37.1406115Z cd /__w/robotology-superbuild/robotology-superbuild/build/src/funny-things && /usr/bin/cmake --build . && /usr/bin/cmake -E touch /__w/robotology-superbuild/robotology-superbuild/build/src/funny-things/CMakeFiles/YCMStamp/funny-things-build
2021-12-22T16:04:37.1407593Z [1/10] Generating code from iCubBlinker.thrift
2021-12-22T16:04:37.1408531Z [2/10] Building CXX object modules/iCubBreather/CMakeFiles/iCubBreather.dir/iCubBreatherModule.cpp.o
2021-12-22T16:04:37.1409786Z [3/10] Building CXX object modules/iCubBreather/CMakeFiles/iCubBreather.dir/iCubBreatherThread.cpp.o
2021-12-22T16:04:37.1410864Z [4/10] Building CXX object modules/iCubBlinker/CMakeFiles/iCubBlinker.dir/main.cpp.o
2021-12-22T16:04:37.1411610Z [5/10] Linking CXX executable bin/iCubBreather
2021-12-22T16:04:37.1412181Z [6/10] Running utility command for copy_lua_in_build
2021-12-22T16:04:37.1412965Z [7/10] Building CXX object modules/iCubBlinker/CMakeFiles/iCubBlinker.dir/iCubBlinker_IDL.cpp.o
2021-12-22T16:04:37.1413921Z FAILED: modules/iCubBlinker/CMakeFiles/iCubBlinker.dir/iCubBlinker_IDL.cpp.o 
2021-12-22T16:04:37.1417364Z /usr/bin/c++  -I/__w/robotology-superbuild/robotology-superbuild/build/src/funny-things/modules/iCubBlinker -isystem /__w/robotology-superbuild/robotology-superbuild/build/install/include -g -std=c++1z -MD -MT modules/iCubBlinker/CMakeFiles/iCubBlinker.dir/iCubBlinker_IDL.cpp.o -MF modules/iCubBlinker/CMakeFiles/iCubBlinker.dir/iCubBlinker_IDL.cpp.o.d -o modules/iCubBlinker/CMakeFiles/iCubBlinker.dir/iCubBlinker_IDL.cpp.o -c /__w/robotology-superbuild/robotology-superbuild/build/src/funny-things/modules/iCubBlinker/iCubBlinker_IDL.cpp
2021-12-22T16:04:37.1420920Z /__w/robotology-superbuild/robotology-superbuild/build/src/funny-things/modules/iCubBlinker/iCubBlinker_IDL.cpp:17:11: error: "'blink_fast' will never be called, since 'blink' starts with the same tag"
2021-12-22T16:04:37.1422404Z  YARP_COMPILER_ERROR("'blink_fast' will never be called, since 'blink' starts with the same tag")
2021-12-22T16:04:37.1423012Z            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2021-12-22T16:04:37.1424418Z /__w/robotology-superbuild/robotology-superbuild/build/src/funny-things/modules/iCubBlinker/iCubBlinker_IDL.cpp:18:11: error: "'blink_naturalistic' will never be called, since 'blink' starts with the same tag"
2021-12-22T16:04:37.1425976Z  YARP_COMPILER_ERROR("'blink_naturalistic' will never be called, since 'blink' starts with the same tag")
2021-12-22T16:04:37.1426627Z            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2021-12-22T16:04:37.1427333Z [8/10] Building CXX object modules/funnyPostures/CMakeFiles/funnyPostures.dir/main.cpp.o
2021-12-22T16:04:37.1428109Z ninja: build stopped: subcommand failed.

fyi @randaz81 @S-Dafarra @pattacini @Nicogene @drdanz

[traversaro]

This was detected in https://github.com/robotology/robotology-superbuild/pull/971 .

[randaz81]

On which branch should I commit the fix? is master ok?

[traversaro]

On which branch should I commit the fix? is master ok?

I guess we need to ask to @pattacini .

[pattacini]

On which branch should I commit the fix? is master ok?

Hi @randaz81, first of all, thanks 👍🏻 The fix should land as a PR against master.

However, it's not clear to me how the fix would look like. Will it allow us to keep the function names equal to blink, blink_fast, and blink_naturalistic? If so, that would be great.

Otherwise, I would do that rather myself as changing the function names would entail verifying the interactions with the possible callers.

[randaz81]

No, unfortunately, the function names cannot be maintened.

I was going to fix it by renaming blink to blink_single.

Another possibility (equally ok, or even better) is to rename blink_fast to blinkFast avoiding the use of the underscore that has a special meaning. In general, I'd recommend always avoiding the use of the underscores.

Please note that the YARP_COMPILER_ERROR was added intentionally to show a compiler error instead of getting a runtime bug (which unfortunately cannot be fixed)

[pattacini]

Ok, nope then. I'll do the job. Thanks anyway.

Another possibility (equally ok, or even better) is to rename blink_fast to blinkFast avoiding the use of the underscore that has a special meaning. In general, I'd recommend always avoiding the use of the underscores.

Gotcha!

[pattacini]

Question.

I was going to fix it by renaming blink to blink_single.

Is this sufficient?

[pattacini]

According to (from https://github.com/robotology/yarp/pull/2780):

WARNING: This makes it impossible to have a service function that starts with the same name as a different function.

So we should interpret that:

• blink + blink_fast is ❌
• blink_single + blink_fast is ✔️

correct?

[randaz81]

yes, correct.

The file to edit is iCubBlinker.thrift

I'd suggest in this case to go for a minimal change (blink_single).

In the future, I recommend going for camel case and avoiding the separator _ which generates a completely different* (bugged) parser. It is not just aesthetics!

*: based on yarp::os::vocabs instead of std::string

[pattacini]

In the future, I recommend going for camel case and avoiding the separator _ which generates a completely different* (bugged) parser. It is not just aesthetics!

*: based on yarp::os::vocabs instead of std::string

If not already mentioned somewhere in the documentation, it'd be great doing so.