Hard coded loop rate

[jlack1987]

Forgive my ignorance if my issue is interpreting the code incorrectly as i'm pretty new to realtime_tools. In this line the loop rate is hard coded to 750Hz. To me this means that if you're running at loop rates greater than 750Hz then you should probably not use this clock tool correct? It seems it would be very easy to make this a configurable argument perhaps with a default rate of 750Hz to not break backwards compatibility.

Additionally i'm a bit confused about the realtime clock implementation here. I'm unable to find anywhere in any documentation here or otherwise that ros::Time::now() is not a real time safe call to make. If it's not safe, then I do see the use of this except the return of getSystemTime is a ros::Time object that, to get time I would still need to call ::now().

Sorry for the questions, but I'm working on a hard real time system so i'm very interested in making sure my method of getting time is a real time safe call. Thanks!

[bmagyar]

Seems like a pretty good point. I personally haven't hit that limit yet.

@graiola , haven't you guys ran stuff on Meka at 1KHz? Perhaps you didn't use this class?

[mathias-luedtke]

I have not found any user on GitHub for this class, only file copies. I am not sure if ros::Time::now() is using a syscall underneath.

However, I don't see any reason why the loop rate should be hard-coded.

[jlack1987]

Just wondering bc I just stumbled upon this class and was a bit surprised to see, what looks to me, a wrapper around ros::Time to make it real time safe when I wasn't aware that it ever was not real time safe. It got me thinking that for my own stuff perhaps I should be swapping out my calls using regular ole ros::Time to using this for better RT performance.

[mathias-luedtke]

it uses gettimeofday, which will make as syscall and so might lead to context switch. I guess this is the reason for passing the time to the update function in ros_control.

[jlack1987]

Ah ok that was what I didn't know, thanks for the explanation. Based on the source code chunk here,

#if HAS_CLOCK_GETTIME
           timespec start;
           clock_gettime(CLOCK_REALTIME, &start);
           sec  = start.tv_sec;
           nsec = start.tv_nsec;
         #else
           struct timeval timeofday;
           gettimeofday(&timeofday,NULL);
           sec  = timeofday.tv_sec;
           nsec = timeofday.tv_usec * 1000;
         #endif

it looks like we are probably ok as long as HAS_CLOCK_GETTIME is defined.

[graiola]

If I remember correctly ros::Time::now(); is not real-time safe. You should use the time implementation provided by your rtos and pass the time information to the update function in ros_control. For example (with RTAI):

const uint64_t one_E9 = 1000000000ULL;
uint64_t nsec64 = rt_get_cpu_time_ns();
uint32_t sec32_part = nsec64 / one_E9;
uint32_t nsec32_part = nsec64 % one_E9;
ros::Time now(sec32_part, nsec32_part);
cm_ptr_->update(now, period_);

[graiola]

I closed it by mistake, sorry.

[jlack1987]

Thanks so much this is really good info, and sorry for sidetracking things from the issue's real purpose. If you all are interested I can make a PR to add that loop rate as an arg with default value of 750.

[bmagyar]

This is something that could potentially be in packages typically not released to the public. This could explain why it is hard to find usage of this. Still... I think adding that param can't hurt.

On Nov 17, 2017 13:55, "jlack1987" notifications@github.com wrote:

Thanks so much this is really good info, and sorry for sidetracking things from the issue's real purpose. If you all are interested I can make a PR to add that loop rate as an arg with default value of 750.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ros-controls/realtime_tools/issues/23#issuecomment-345249930, or mute the thread https://github.com/notifications/unsubscribe-auth/ADXH4afjcMfQPCSmOcEBSyhI99cIZIISks5s3ZBpgaJpZM4QhRCg .

[JimmyDaSilva]

Hi folks. I see the 750Hz hard-coded value is still here. Should we do something about this? Add a parameter, or up the value to 1 kHz?

[bmagyar]

Happy to review a PR to add that loop rate as an arg with default value of 750.