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

Long delay before starting trajectory streaming #191

Closed
simonschmeisser opened this issue Mar 23, 2018 · 8 comments
Closed

Long delay before starting trajectory streaming #191

simonschmeisser opened this issue Mar 23, 2018 · 8 comments

Comments

@simonschmeisser
Copy link
Contributor

ros::Duration(0.250).sleep(); // slower loop while waiting for new trajectory

We wondered about long delays before trajectory streaming starts and there is actually a (up to) 250ms sleep before the sending thread goes into action. Is there any reason for it? I guess the idea was to avoid busy looping but that could also work with a shorter sleep.

I'll try to write a PR that uses a condition variable instead. Are we on C++98/03 still?

@gavanderhoorn
Copy link
Member

gavanderhoorn commented Mar 23, 2018

250ms is not really that long (the robot probably takes longer to start moving, even if there was 0 ms sleep time). Are you sure you're not running into ros-industrial/abb#142?

@simonschmeisser
Copy link
Contributor Author

Well, true, but it adds up in the end. We're currently looking at the old/default fanuc driver and admittedly there are bigger issues but 250ms wasted is already a big issue in itself. A typical cycle of our application right now consists of 8 trajectories so that is 2sec (worst case ...) wasted already.

@gavanderhoorn
Copy link
Member

Sure. Lowering the sleep to 10ms or something would be fine.

@simonschmeisser
Copy link
Contributor Author

It looks like industrial_core is not yet on c++11, am I correct on that? Would it be possible to switch? This would allow a non-polling version based on std::condition_variable

@gavanderhoorn
Copy link
Member

No, we're not using C++11 for this package yet.

The minimal patch would be to lower the period. CPU usage increase should be minor (if at all really).

If you really want to, Boost has condition variables as well, so you could use those.

@davetcoleman
Copy link
Contributor

I ran into the no C++11 thing yesterday. It seems to me a simple no-brainer to modify the CMakeList.

@pbeeson
Copy link
Contributor

pbeeson commented Nov 1, 2018

I recall lowering the 250ms period in a previous PR and people didn't like that and didn't merge the PR. We found 250ms to be prohibitive. If you have Let's say 20 trajectories in an industrial tasks, that's 5 seconds to the cycle time (worst case). In my fork, we made it 100ms and see no increase in overall CPU usage but a measurable difference in tasks that use many trajectories, but think 10ms should be fine.

@pbeeson
Copy link
Contributor

pbeeson commented Nov 1, 2018

Sorry it was PR #174 where it was set to 100ms. There are other changes here too, but this PR was never accepted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants