fix check motion done + remove warning

[valegagge]

In this PR, I try to fix the check motion done error and remove the annoying warnings:

I also added a comment for @randaz81 in the code: I think it’s better if checkMotionDone returns false if the function fails. What do you think?

cc @martinaxgloria

[update-docs]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would update the release notes by adding a file in doc/release/<target_branch>, based on your changes.

[gemini-code-assist]

Summary of Changes

Hello @valegagge, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses two main areas: reducing noisy log output and refining error handling for device interfaces. It comments out several recurring warning/error messages to declutter the console and elevates the severity of a missing iJointBrake interface to ensure critical setup issues are more prominently reported. Additionally, it captures the return status of motion done checks for improved diagnostic capabilities.

Highlights

• Warning Suppression: Several yCError and yWarning messages related to isJointBraked and checkMotionDone failures have been commented out to reduce console output.
• Error Severity Adjustment: The logging level for the absence of the iJointBrake interface in ControlBoard_nws_yarp has been upgraded from a warning (yCWarning) to an error (yCError).
• Motion Done Check Logic: The return value of m_iPos->checkMotionDone is now explicitly captured in a variable ret_motdone in PartItem::updatePart(), potentially for further processing or debugging.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog

• src/devices/networkWrappers/RemoteControlBoard/RemoteControlBoard.cpp
  • Commented out an yCError message that was triggered when isJointBrakedRPC failed.
• src/devices/networkWrappers/controlBoard_nws_yarp/ControlBoardServerImpl.cpp
  • Commented out an yCError message for isJointBraked() failures.
• src/devices/networkWrappers/controlBoard_nws_yarp/ControlBoard_nws_yarp.cpp
  • Changed yCWarning to yCError when the iJointBrake interface is not found in a subdevice.
• src/guis/yarpmotorgui/partitem.cpp
  • Captured the return value of m_iPos->checkMotionDone into ret_motdone.
  • Commented out a yWarning message regarding missing isJointBraked() implementation.

Activity

• The author initiated this PR to fix a 'check motion done' error and eliminate 'annoying warnings' as stated in the PR description.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request aims to fix an issue with checkMotionDone and remove some warnings. While the changes address the symptoms, some of them involve commenting out error and warning logs, which is not ideal as it can hide underlying problems. I've suggested alternative approaches, such as handling failures gracefully in the GUI to avoid repeated calls to unavailable functions, instead of silencing logs. Additionally, there's a critical issue in partitem.cpp where the return value of checkMotionDone is not checked before using its output parameter. This could lead to incorrect state representation in the GUI. I've provided a suggestion to fix this. The change from yCWarning to yCError for a missing iJointBrake interface is a good improvement, making setup issues more visible.

[gemini-code-assist]

The value of the output parameter boolval is only guaranteed to be valid if checkMotionDone returns a success code. The current implementation assigns boolval to m_done[m_slow_k] regardless of the return value, which can lead to an incorrect state if the call fails. The assignment should be conditional on the success of the function call. Also, the comment left for @randaz should be removed and replaced with the correct logic.

        ret_motdone = m_iPos->checkMotionDone(m_slow_k, &boolval); // using k to save bandwidth
        if (ret_motdone)
        {
            m_done[m_slow_k] = boolval;
        }

[gemini-code-assist]

Commenting out error logs is generally not a good practice as it can hide underlying issues and make debugging more difficult. If this error is being triggered frequently for non-critical reasons (e.g., an optional interface not being present), consider downgrading it to yCWarning or yCDebug. Alternatively, the calling code could handle this failure more gracefully, perhaps by checking for the interface's availability once and then avoiding repeated calls.

        yCError(REMOTECONTROLBOARD, "Unable to isJointBraked");

[gemini-code-assist]

Silencing error logs can make it difficult to diagnose problems. This log indicates a failure in the underlying isJointBraked() implementation. If this failure is expected in some cases (e.g., the interface is optional), it might be better to handle it by logging at a lower severity level like yCWarning or yCDebug. Simply commenting it out hides the issue entirely.

Suggested change

	//yCError(CB_RPC, "isJointBraked() failed");
	yCError(CB_RPC, "isJointBraked() failed");

[gemini-code-assist]

Commenting out this warning hides a recurring issue. If isJointBraked() is failing because the feature is not available on some hardware, the GUI should detect this and avoid calling it repeatedly in the update loop. A better approach would be to log the warning once and then set a flag to disable future calls for that part. For example, you could add a member variable m_jointBrakeIsAvailable to PartItem, initialized to true. If isJointBraked() fails, set this flag to false and log a warning. In subsequent updates, only call isJointBraked() if the flag is true.

        yWarning() << "Missing Implementation of isJointBraked()";