Improved tests

[randaz81]

improved tests to check unimplemented methods in device drivers

[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 @randaz81, 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 significantly strengthens the testing framework for YARP device drivers by introducing a dedicated mechanism to verify the handling of unimplemented methods. It ensures that both local and network-wrapped fake devices can correctly signal and respond to calls for non-existent functionalities, thereby improving the robustness and predictability of device interactions within the YARP ecosystem. Additionally, it expands the RPC capabilities of the FakeDeviceWrapper for more thorough testing.

Highlights

• New Unimplemented Method for Testing: A new method, doSomethingUnimplemented, has been introduced to the IFakeDeviceInterfaceTest1 interface and its concrete implementation FakeDeviceUnwrapped. This method is designed to explicitly return an return_value_error_not_implemented_by_device to facilitate testing of how clients handle unimplemented functionalities.
• RPC Support for Unimplemented Methods: The FakeDevice_nwc_yarp (network wrapper client) and FakeDeviceServerImpl (network wrapper server) have been updated to include RPC support for the new doSomethingUnimplemented method. This allows for comprehensive testing of network-wrapped devices' behavior when encountering unimplemented interface methods.
• Enhanced FakeDeviceWrapper RPC Capabilities: The FakeDeviceWrapper now incorporates an RPC port and implements the PortReader interface. This enhancement enables it to process incoming RPC commands for testSetValue and testGetValue, improving its testability and interaction capabilities via RPC.

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 introduces a new method doSomethingUnimplemented to test the handling of unimplemented functions in device drivers, and also adds RPC capabilities to FakeDeviceWrapper. The changes for doSomethingUnimplemented are well-implemented across the different device types (unwrapped, nwc, nws).

My review focuses on the new RPC implementation in FakeDeviceWrapper, where I've identified and suggested fixes for a few issues, including a hardcoded value, a missing return value in the reply, and lack of input validation. I've also suggested a couple of minor refactorings to improve code clarity and maintainability.

[gemini-code-assist]

The RPC command handler in the read method has several issues that should be addressed:

• Hardcoded value: The testSetValue command uses a hardcoded value of 1 instead of parsing it from the command bottle.
• Missing return value: The testGetValue command does not include the retrieved value in the reply bottle.
• Lack of validation: There are no checks for the number of arguments or their types, which can lead to runtime errors.
• Code style: The else if formatting is unconventional.

I suggest refactoring this block to make it more robust, correct, and readable.

    if (command.get(0).asString() == "testSetValue")
    {
        if (command.size() >= 2 && command.get(1).isInt32()) {
            retval = testSetValue(command.get(1).asInt32());
            reply.addVocab32(retval ? VOCAB_OK : VOCAB_ERR);
        } else {
            yCError(FAKEDEVICEWRAPPER) << "testSetValue command requires an integer argument.";
            reply.addVocab32(VOCAB_ERR);
        }
    }
    else if (command.get(0).asString() == "testGetValue")
    {
        int val = 0;
        retval = testGetValue(val);
        if (retval) {
            reply.addVocab32(VOCAB_OK);
            reply.addInt32(val);
        } else {
            reply.addVocab32(VOCAB_ERR);
        }
    }
    else
    {
        yCWarning(FAKEDEVICEWRAPPER) << "Unknown command:" << command.toString();
        reply.addVocab32(VOCAB_ERR);
    }

[gemini-code-assist]

The header <yarp/dev/ControlBoardInterfaces.h> is included but not used in this file. It should be removed to keep the dependencies clean and potentially improve compilation times.

[gemini-code-assist]

The implementation of doSomethingUnimplementedRPC is functionally correct, but it can be simplified for better readability and maintainability. The logic of checking the return value and then returning can be made more concise. Also, the error message could be more informative by including the reason for failure.

Suggested change

	yarp::dev::ReturnValue ret = m_iFake->doSomethingUnimplemented();
	if (!ret)
	{
	yCError(FAKEDEVICESERVER, "doSomethingUnimplemented() failed");
	return ret;
	}
	return yarp::dev::ReturnValue_ok;
	yarp::dev::ReturnValue ret = m_iFake->doSomethingUnimplemented();
	if (!ret)
	{
	yCError(FAKEDEVICESERVER) << "doSomethingUnimplemented() failed with error:" << ret.toString();
	}
	return ret;