add status_rate option as we had in rosmake #141
Conversation
|
Thanks for the pull request, I'll have a look at it as soon as possible. Also, it looks like there are some failing tests related to PEP8 compliance. |
k-okada
force-pushed
the
add_status_rate_option
branch
from
0507dd7 to
655abe6
Compare
k-okada
force-pushed
the
add_status_rate_option
branch
from
655abe6 to
dc62a99
Compare
|
fixed style ◉ Kei Okada On Wed, Jan 28, 2015 at 12:55 PM, William Woodall notifications@github.com
|
| @@ -215,6 +217,7 @@ def main(opts): | |||
| quiet=not opts.verbose, | |||
| interleave_output=opts.interleave_output, | |||
| no_status=opts.no_status, | |||
| status_update_rate=opts.status_update_rate, | |||
There was a problem hiding this comment.
We should ensure that the given status update rate is positive and non-zero.
There was a problem hiding this comment.
Also, we should put a limit on it based on the update loop rate, which is currently running at 10Hz:
https://github.com/catkin/catkin_tools/pull/141/files#diff-2482c854781a7d39252bf95c9201c631R521
Because as is, if the user puts --status-rate=20 it will still only run at 10Hz.
There was a problem hiding this comment.
check these case in 6c682ed
k-okada
force-pushed
the
add_status_rate_option
branch
from
277e642 to
eca90f9
Compare
|
passed the test |
|
I want to do the check differently, but I'll merge it manually with the appropriate changes. |
|
I merged your commits in 70bbe0b and touched them up in 6997625. The option now allows you to set 0 to turn off limiting (as fast as normal), and there is no upper limit. I did this because it actually ended up working differently that I had first imagined. I also renamed the option to Anyways, give master a spin and see if it is ok with you. |
|
it works find for me! thanks. ◉ Kei Okada On Wed, Feb 4, 2015 at 10:58 AM, William Woodall notifications@github.com
|
jbohren
modified the milestones:
x.x.x - Untargeted Features,
0.4.0 - Second Beta Release
No description provided.