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

Yarp rfmodule plugin #1532

Merged
merged 5 commits into from
Feb 11, 2019
Merged

Yarp rfmodule plugin #1532

merged 5 commits into from
Feb 11, 2019

Conversation

aerydna
Copy link
Collaborator

@aerydna aerydna commented Feb 6, 2018

The following pr adds the new type of plugin RFModule.

After including RFModuleFactory.h and creating an instance of RFPlugin will be possible to load dinamically or statically an RFModule calling RFPlugin::open(string commandline).

On the other side, to generate an RFModule plugin will be enough to use the usual yarp_prepare_plugin and yarp_add_plugin macro

The purpose is to give the possibility of running existing modules in a single process contest.

@aerydna aerydna requested a review from drdanz February 6, 2018 19:02
@aerydna aerydna added the PR Status: Changelog - Not OK This PR does not have a changelog entry, or the changelog needs to be fixed label Feb 6, 2018
@aerydna
Copy link
Collaborator Author

aerydna commented Feb 7, 2018

i don't know why, but for some reasons the pr is not aligned with the branch on my remote. In particular a file was renamed from yarp_plugin_RFModule.cpp.in to yarp_plugin_rfmodule.cpp.in but in this pr exists both version.

i suspect the problem to be related to the warning on [https://help.github.com/articles/about-pull-requests/](this page) wich says When pushing commits to a pull request, don't force push. Force pushing can corrupt your pull request.

the fact is that i didn't push on the pull request branch (in origin) but on the one on my remote and reading the statement isn't clear on which branch is forbidden the force push.

@robotology robotology deleted a comment Feb 7, 2018
@robotology robotology deleted a comment Feb 13, 2018
@robotology robotology deleted a comment Feb 13, 2018
@robotology robotology deleted a comment Feb 15, 2018
@robotology robotology deleted a comment Mar 27, 2018
@aerydna aerydna changed the title [WIP] Yarp rfmodule plugin Yarp rfmodule plugin Mar 27, 2018
@aerydna aerydna closed this Jun 27, 2018
@aerydna aerydna reopened this Jun 27, 2018
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.

Can you please add a dummy example that implements an RFModule as plugin?
It would be really useful for reviewing the PR.

src/libYARP_OS/include/yarp/os/RFModuleFactory.h Outdated Show resolved Hide resolved
src/libYARP_OS/include/yarp/os/RFModuleFactory.h Outdated Show resolved Hide resolved
@drdanz
Copy link
Member

drdanz commented Oct 3, 2018

@aerydna ping. This PR is in a pretty good shape, but it still needs documentation and examples, do you think you can find some time to take care of that?

@Nicogene Nicogene added PR Type: Feat/Enh This PR adds some new feature or enhances some part of YARP and removed Target: Future labels Oct 3, 2018
doc/release/v3_1_0.md Outdated Show resolved Hide resolved
Copy link
Member

@Nicogene Nicogene left a comment

Choose a reason for hiding this comment

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

Please fix the changelog.

@aerydna
Copy link
Collaborator Author

aerydna commented Oct 3, 2018

i'll try to find some time for finishing the pr but i cannot guarantee when i'll do it. it may be postponed to late november for the examples part. other stuff are quick

@aerydna aerydna force-pushed the yarp_rfmodule_plugin branch 2 times, most recently from 358e0b2 to 2097ba2 Compare December 21, 2018 16:38
@aerydna aerydna force-pushed the yarp_rfmodule_plugin branch 4 times, most recently from 392c864 to 8bbefd4 Compare January 9, 2019 13:27
@aerydna aerydna removed the PR Status: Changelog - Not OK This PR does not have a changelog entry, or the changelog needs to be fixed label Jan 9, 2019
Copy link
Member

@Nicogene Nicogene left a comment

Choose a reason for hiding this comment

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

LGTM

@drdanz
Copy link
Member

drdanz commented Feb 11, 2019

Rebased and updated copyright.

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

@drdanz drdanz merged commit 3c9226c into robotology:devel Feb 11, 2019
@aerydna aerydna deleted the yarp_rfmodule_plugin branch March 26, 2019 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants