Posted by @cwrx777:
Refer to here
Robot which has a lift destination publishes lift request when receiving lift states for other lifts.
Posted by @mxgrey:
That publication happens at the end of the function to ensure that any possible changes to the robot’s lift request that might happen within the function will be published before it returns. Otherwise we would need to carefully identify each code path that potentially changes the request and make sure to publish for each case.
The only downside would be that there are more request messages being published than what’s strictly needed, but the lift supervisor should ensure that the overall system behavior is stable. Are you seeing this message being published excessively to the point that it’s detrimental to performance?
Posted by @cwrx777:
In our current implementation there are more than 10 lifts and each lift publishes lift state at 1 Hz and this line is flooding the lift supervisor log file.
is it better if it’s implemented as follows?
if (_lift_destination && _lift_destination->lift_name == state.lift_name)
{
_publish_lift_destination();
}
Posted by @mxgrey:
If the only concern is the observable effect on the log for the lift supervisor then I’d suggest we change that specific log to RCLCPP_DEBUG. We can change the info log here so that it only prints out when the lift state was different from the request that’s being sent. That would ensure you have a log for each time the requests going to the lift get changed.
Posted by @cwrx777:
I think my question is really is it necessary for a robot to publish lift request for the lift that is requesting when it receives lift state for other lifts that it is not requesting. if these extra lift requests were filtered out, lift supervisor could better use its time to handle an actual request (adapter lift request when a robot receives lift state of the lift it’s currently requesting).
Posted by @mxgrey:
Roughly speaking the reason is to guarantee eventual consistency. In the early days of developing RMF we saw a lot of unreliability from DDS where messages would get dropped even with reliable QoS and large buffer sizes. The current aggressive publishing approach ensures that even if many messages get dropped, the system will eventually do what we intend it to do.
We could probably find ways to make this more efficient, but we need to be very cautious or else we may introduce edge cases where a request doesn’t go out even though it’s needed.
If someone finds that the level network activity is unreasonable then it will be worth examining further. If anyone would like to look for ways to improve the network efficiency for the current protocol then I’ll be happy to review a PR for it. Eventually we’ll open a community discussion for a next generation protocol that will aim to be more efficient.
In the meantime I’ve opened this PR which I believe should at least reduce the amount of churn in the log files: Reduce churn in lift supervisor by mxgrey · Pull Request #418 · open-rmf/rmf_ros2 · GitHub
Posted by @cwrx777:
@mxgrey
Thanks for the PR. At the moment we’ve added the following condition before calling _publish_lift_destination().
if (_lift_destination && _lift_destination->lift_name == state.lift_name)
{
_publish_lift_destination();
}
Posted by @mxgrey:
That’s a reasonable filter. I’ll add that into the PR.
I didn’t want to do that initially because I thought it would interfere with cases where we need to release the lift, but I see now that there’s a separate code path for that.
Edited by @mxgrey at 2025-04-22T11:38:05Z