Skip to content

Conversation

@MSECode
Copy link
Contributor

@MSECode MSECode commented Dec 10, 2025

This PR fixes the configuration files for iCubGenova11 in relation to the rawValuesPublisherRemapper. This should fix the CI as well

@MSECode MSECode self-assigned this Dec 10, 2025
@gemini-code-assist

This comment was marked as outdated.

@MSECode MSECode requested a review from pattacini December 10, 2025 17:46
gemini-code-assist[bot]

This comment was marked as outdated.

@pattacini
Copy link
Member

Hi @MSECode

The CI (xml) still fails for iCubGenova11.
We can handle that tomorrow!

@pattacini

This comment was marked as resolved.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request correctly updates the configuration files to use rawValuesPublisherRemapper for iCubGenova11, and adjusts the CI checks accordingly. The changes are logical and address the goal of the PR. I have a few suggestions to improve the code quality, mainly related to consistency and maintainability. I've pointed out some missing newlines at the end of files, a misleading log message, and an opportunity to reduce code duplication. Overall, good work on fixing the configuration and CI.

@MSECode
Copy link
Contributor Author

MSECode commented Dec 11, 2025

So, in order to fix the CI, we needed to update the check-xml due to the following problem:

  • when working with devices that are not of type motorControl, e.g the rawValuesPublisherServer or rawValuesPublisherRemapper devices, the pipeline of the CI that checks the wrappers and remappers cannot follow the standard path.
  • Specifically, the main issue is related to the fact that the target that is used to subsequently check the wrappers and remappers saved in the wrappers/motorControl folder is taken at the beginning of the pipeline as the value of the target node in the calibrator file as shown at this line
  • therefore, if we'd like to check any wrapper and remapper that is not defined in the calibrator file, we needed to update the code of the ctest application

Thus, the main issue that made the CI failing was the fact that for iCubGenova11 we introduced the raw values publisher wrappers and remappers in the wrapper/motorControl folder. However, those remappers are different from the ones defined in the calibrator file.
So, at the current status those wrappers are saved to a different list from the one of the motor control devices and at the end we perform a specific check for the not motor control devices by checking that the specific remapper is correctly defined in the related wrapper.
For the future, we can improve the current ctest application in order to make it more robust by generalizing the wrapper check for any not motor control device (considering that others might be introduced), adding also the check of the control boards defined in the remapper.

@pattacini
Copy link
Member

Thanks for the extensive explanation, @MSECode 🚀
Merging the PR.

@pattacini pattacini merged commit 4529db8 into robotology:master Dec 11, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants