Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move yarprobotinterface logic to a standalone libYARP_robotinterface library #2168

Merged
merged 12 commits into from
Jan 10, 2020

Conversation

traversaro
Copy link
Member

@traversaro traversaro commented Jan 4, 2020

This PR fixes the first part of #2005 . The possibility of attaching to device not opened by the yarp::robotinterface::Robot object will be added later.

The following snippet show a basic use of the new libYARP_robotinterface library:

// Load the XML configuration file 
std::string pathToXmlConfigurationFile = ...;
yarp::robotinterface::XMLReader reader;
yarp::robotinterface::Robot robot  = reader.getRobot(pathToXmlConfigurationFile);

bool ok = true;

// Enter the startup phase 
ok = robot.enterPhase(yarp::robotinterface::ActionPhaseStartup);

if (!ok) {
    // Handle error 
    // ...
}

// Enter the run phase
ok = robot.enterPhase(yarp::robotinterface::ActionPhaseRun);

if (!ok) {
    // Handle error 
    // ...
}

// At this point, the system is running and the thread that called the 
// enterPhase methods does not need to do anything else 

// When you want to close the yarprobotinterface 

// Enter interrupt phase 
ok = robot.enterPhase(yarp::robotinterface::ActionPhaseInterrupt1);
if (!ok) {
    // Handle error 
    // ...
}

There are the following open points, but I would prefer to get an early review on the modifications, so that this points can be addressed considering also the early review: All the open points have been addressed

  • Tests are missing (at least a basic test to run valgrind on the classes would be nice) Basic test for a library class added in ec5629f
  • The RobotInterfaceDTD class uses a TinyXML pointer as part of its public interface. At the moment there is no explicit dependency on the TinyXML thanks to the forward declaration added in 5403225, but in general to make the class actually usable from outside it would be great to avoid the use of TinyXML classes. If this is not possible, probably it would make sense to make the RobotInterfaceDTD a non-public class, making its headers private. This was fixed my making RobotInterfaceDTD a private header.

@traversaro
Copy link
Member Author

cc @prashanthr05 @diegoferigo

@traversaro
Copy link
Member Author

@drdanz there are a lot of failing tests for the licence check, any idea? It could be related to the -2020 in the headers?

1: [NOT OK (unknown license)] src/libYARP_robotinterface/CMakeLists.txt
1: [NOT OK (unknown license)] src/libYARP_robotinterface/src/CMakeLists.txt
1: [NOT OK - LGPL2.1+ (library robotinterface)] src/libYARP_robotinterface/src/yarp/robotinterface/Action.cpp
1: [NOT OK - LGPL2.1+ (library robotinterface)] src/libYARP_robotinterface/src/yarp/robotinterface/Action.h
1: [NOT OK - LGPL2.1+ (library robotinterface)] src/libYARP_robotinterface/src/yarp/robotinterface/CalibratorThread.cpp
1: [NOT OK - LGPL2.1+ (library robotinterface)] src/libYARP_robotinterface/src/yarp/robotinterface/CalibratorThread.h
1: [NOT OK - LGPL2.1+ (library robotinterface)] src/libYARP_robotinterface/src/yarp/robotinterface/Device.cpp
1: [NOT OK - LGPL2.1+ (library robotinterface)] src/libYARP_robotinterface/src/yarp/robotinterface/Device.h
1: [NOT OK - LGPL2.1+ (library robotinterface)] src/libYARP_robotinterface/src/yarp/robotinterface/Param.cpp
1: [NOT OK - LGPL2.1+ (library robotinterface)] src/libYARP_robotinterface/src/yarp/robotinterface/Param.h
1: [NOT OK - LGPL2.1+ (library robotinterface)] src/libYARP_robotinterface/src/yarp/robotinterface/Robot.cpp
1: [NOT OK - LGPL2.1+ (library robotinterface)] src/libYARP_robotinterface/src/yarp/robotinterface/Robot.h
1: [NOT OK - LGPL2.1+ (library robotinterface)] src/libYARP_robotinterface/src/yarp/robotinterface/RobotInterfaceDTD.cpp
1: [NOT OK - LGPL2.1+ (library robotinterface)] src/libYARP_robotinterface/src/yarp/robotinterface/Thread.h
1: [NOT OK - LGPL2.1+ (library robotinterface)] src/libYARP_robotinterface/src/yarp/robotinterface/Types.cpp
1: [NOT OK - LGPL2.1+ (library robotinterface)] src/libYARP_robotinterface/src/yarp/robotinterface/Types.h
1: [NOT OK - LGPL2.1+ (library robotinterface)] src/libYARP_robotinterface/src/yarp/robotinterface/XMLReader.h
1: [NOT OK - LGPL2.1+ (library robotinterface)] src/libYARP_robotinterface/src/yarp/robotinterface/XMLReaderV1.cpp
1: [NOT OK - LGPL2.1+ (library robotinterface)] src/libYARP_robotinterface/src/yarp/robotinterface/XMLReaderV3.cpp
1: [NOT OK - LGPL2.1+ (library robotinterface)] src/libYARP_robotinterface/src/yarp/robotinterface/XMLReaderVx.cpp
1: [NOT OK (unknown license)] src/libYARP_robotinterface/src/yarp/robotinterface/api.h
1: [NOT OK - LGPL2.1+ (library robotinterface)] src/libYARP_robotinterface/src/yarp/robotinterface/impl/RobotInterfaceDTD.h
1: [NOT OK (unknown license)] tests/libYARP_robotinterface/CMakeLists.txt

@drdanz
Copy link
Member

drdanz commented Jan 8, 2020

Yes, the 2020 is not recognized yet, let me take care of that...

@drdanz
Copy link
Member

drdanz commented Jan 8, 2020

Right, I forgot... It's still failing because libraries are expected to be BSD.
I see no reasons for keeping it LGPL, @lornat75 do you agree with this change?

@drdanz
Copy link
Member

drdanz commented Jan 8, 2020

Rebased and changed to BSD.
I'll wait for @lornat75 authorization before merging it

@drdanz drdanz added Component: Tool - yarprobotinterface PR Status: Copyright - Not OK Files in this PR have copyright issues Target: YARP v3.4.0 PR Type: Feat/Enh This PR adds some new feature or enhances some part of YARP labels Jan 8, 2020
@robotology robotology deleted a comment from traversaro Jan 8, 2020
@robotology robotology deleted a comment from traversaro Jan 8, 2020
@lornat75
Copy link
Member

lornat75 commented Jan 8, 2020

ok for changing the license!

@drdanz drdanz added PR Status: Copyright - OK Files in this PR don't have copyright issues and removed PR Status: Copyright - Not OK Files in this PR have copyright issues labels Jan 9, 2020
Copy link
Member

@drdanz drdanz left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Tool - yarprobotinterface PR Status: Copyright - OK Files in this PR don't have copyright issues PR Type: Feat/Enh This PR adds some new feature or enhances some part of YARP Resolution: Merged Target: YARP v3.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants