yarp
yarp copied to clipboard
PortReport deprecated in favor of modern callbacks
the pr makes possible to pass a callable to everywhere the PortReport was used.
example
port.setReportCallback([ ](const yarp::os::PortInfo & info) { //do things });
the major difference when passing a functor that has member variables that changes inside the operator() is that its required to wrapping it with std::ref()
I don't agree, having to declare a class just to add a callback is confusing and very suboptimal.
Just to understand, it would be helpful to provide an example of how an existing piece of code that uses a class that inherits from PortReport will change with this new pattern.
@drdanz my point is that the class might be responsible for the callback plus some other handling; it very much depends on what type of computation is required.
Examples of this new usage that cover a wide range of cases may enrich the PR.
Marking with 👎 anyone who doesn't agree with you is not a common attitude in our community @drdanz
Marking with :-1: anyone who doesn't agree with you is not a common attitude in our community
That was not personal, you proposed Why don't we add up this new functionality on top without deprecating the good old method? and I expressed my opinion against that proposal.
my point is that the class might be responsible for the callback plus some other handling; it very much depends on what type of computation is required.
This can still be done by passing a member of the class as callback
my point is that the class might be responsible for the callback plus some other handling; it very much depends on what type of computation is required.
This can still be done by passing a member of the class as callback
Just to understand, this means that:
PortReportChild callbackFunctor;
port.setReportCallback(callbackFunctor);
becomes:
PortReportChild callbackFunctor;
port.setReportCallback(std::bind(&PortReportChild::report, &callbackFunctor, _1));
?
@drdanz, of course, that wasn't personal, I was referring to a different context and attitude.
Anyway, my major concern was about the in-place definition of the callback as it was remarked from the example given in the PR description.
Lack of clarity in the PR description generates confusion and misunderstanding that we can easily avoid by providing examples – or at least more details – as already mentioned. This is because we don't have always time to review/peruse the PR content.
Either std::bind or (better) using a lambda:
port.setReportCallback([](const yarp::os::PortInfo& info){ PortReportChild::report.callbackFunctor(info); });
Either
std::bindor (better) using a lambda:port.setReportCallback([](const yarp::os::PortInfo& info){ PortReportChild::report.callbackFunctor(info); });
I guess if you have an instance of PortReportChild , you should do as:
port.setReportCallback([&callbackFunctor](const yarp::os::PortInfo& info){ callbackFunctor.report.callbackFunctor(info); });
@drdanz, of course, that wasn't personal, I was referring to a different context and attitude.
Again, I have no idea what you are talking about. :-1: is just a reaction to a comment. If we have some misunderstanding, feel free to come talk to me.
I agree that some example might be useful, but nonetheless, this is a very basic C++11 callback approach that has been around for about 9 years. It's more or less the same that is used in almost any std algorithm, I don't think it requires a big explanation.
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.
:white_check_mark: drdanz
:x: aerydna
You have signed the CLA already but the status is still pending? Let us recheck it.
closing for obsolescence