WIP Gopher bugfix - Githubissues

pyros-dev / pyros

Python interfacing for multiprocess software - A Python blanket for ROS to hide inside

BSD 3-Clause "New" or "Revised" License

24 stars 4 forks source link

WIP Gopher bugfix #103

Closed asmodehn closed 7 years ago

asmodehn commented 7 years ago

A set of bugfix for annoying things for gopher :

[x] ROS Param interface fixes
[x] Fixing slow service/topics interfacing (one per update), even if discovery is fast...
[ ] ~~Exposing Timeout to client to allow user customization~~ => LATER
[x] Investigate topics conflict with gateway behavior, in case a simple fix is possible (list endpoints ?) relates to https://github.com/asmodehn/rostful/issues/92

And, if time allows, a new version release after that. Required to stabilize things.

asmodehn commented 7 years ago

note : Travis failed because of timeouts when using ROS... might be an opportunity to optimize things ?

asmodehn commented 7 years ago

Ah no thats because pyros_test package hasn't been release with changes yet...

stonier commented 7 years ago

Do you want to merge this in and work on the remaining two issues separately? Solves the first problem for me.

asmodehn commented 7 years ago

I still need to solve the problem of deleting topics for multiple pyros instance (our case for concert). After that we can merge it. I ll let you know.

On Oct 26, 2016 4:45 PM, "Daniel Stonier" notifications@github.com wrote:

Do you want to merge this in and work on the remaining two issues separately? Solves the first problem for me.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/asmodehn/pyros/pull/103#issuecomment-256273581, or mute the thread https://github.com/notifications/unsubscribe-auth/AANgSONWsJf8xrPLC_SnhpTpIRq_ofu1ks5q3wUqgaJpZM4KejaO .

stonier commented 7 years ago

asmodehn commented 7 years ago

@stonier This version is managing topic/service/param going up and down much better than before. I am using the ros param server to keep track of multiple pyros instances.

But it still has issues with user nodes (rostful/celeros) going down (does not cleanup). We would need to review how they start and are killed, signal handling and all... https://github.com/asmodehn/pyros/issues/105

But as this is not in our usecase for now I think we can merge this and start using it.

asmodehn commented 7 years ago

We might have a performance issue... on a gopher v2.0 robot, CPU usage oscillate between 15% to 25 % (cache enabled). I ll need to investigate more...

asmodehn commented 7 years ago

After some profiling (multiple nodes with pubs coming up and down, while looping rosinterface update, with and without connection cache), I couldn't find any obvious bottleneck in the code. So I think we should merge this and move forward. Ultimately one of the two pyros instance will be gone from the robot.

asmodehn commented 7 years ago

This seems finally ready for use, after a lot of improvements. podium simulation working OK. Testing on actual gopher robot to validate performance improvements :

@stonier with gopher_concert (cache enabled) :

rostful [1..3]%CPU
celeros [1..3]%CPU
gateway [2..6]%CPU

So it s quite nice now :-)