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

Time syncronization plugin #156

Closed
vooon opened this issue Sep 12, 2014 · 8 comments
Closed

Time syncronization plugin #156

vooon opened this issue Sep 12, 2014 · 8 comments

Comments

@vooon
Copy link
Member

vooon commented Sep 12, 2014

@mhkabir described main steps for time sync. But rather than doing that in sys_status i'll create new one: sys_time.

Also ROS API for gps will be changed, initially it copy nmea_navsat_driver package interface.
But in that release i don't spread usage on other plugins.

Related #155, #95.

@vooon vooon added this to the Version 0.8.0 milestone Sep 12, 2014
@vooon vooon mentioned this issue Sep 12, 2014
4 tasks
@mhkabir
Copy link
Member

mhkabir commented Sep 12, 2014

Please note, offset is calculated as (this system) - (other system). Offset added to all message timestamps which come from PX4 side(time_boot_ms) to get them into ROS time.

@vooon vooon modified the milestones: Release 0.9.0, Version 0.8.0 Sep 22, 2014
@vooon
Copy link
Member Author

vooon commented Sep 22, 2014

Moved to 0.9.

@mhkabir
Copy link
Member

mhkabir commented Oct 6, 2014

Done in #159
Code is untested till now, need to run it on a MAV.

vooon added a commit that referenced this issue Oct 7, 2014
Also reduce class variables count (most not used outside the method).

Issue #156.
vooon added a commit that referenced this issue Oct 7, 2014
TODO: implement fcu_time().
Issue #156.
vooon added a commit that referenced this issue Oct 8, 2014
@mhkabir
Copy link
Member

mhkabir commented Oct 8, 2014

@vooon As per new changes on PX4 side, we do everything in usec (microseconds)

Can you please update all ms to us and only convert if required (e.g mtime.time_boot_ms*1000)

vooon added a commit that referenced this issue Oct 8, 2014
@mhkabir
Copy link
Member

mhkabir commented Oct 9, 2014

@vooon Can you check this please :

[ 94%] Building CXX object mavros/mavros/CMakeFiles/mavros_plugins.dir/src/plugins/sys_time.cpp.o
/root/catkin_ws/src/mavros/mavros/src/plugins/sys_time.cpp: In member function 'void mavplugin::SystemTimePlugin::handle_system_time(const mavlink_message_t*, uint8_t, uint8_t)':
/root/catkin_ws/src/mavros/mavros/src/plugins/sys_time.cpp:197:68: warning: integer overflow in expression [-Woverflow]
   const bool fcu_time_valid = mtime.time_unix_usec > 1234567890L * 1000000;
                                                                    ^
/root/catkin_ws/src/mavros/mavros/src/plugins/sys_time.cpp:198:54: warning: integer overflow in expression [-Woverflow]
   const bool ros_time_valid = now_ms > 1234567890L * 1000;

Just a warning but worth fixing/cleaning up.

@LorenzMeier
Copy link
Member

The right constant for 64 bit integers is ULL (for unsigned long long).

@mhkabir
Copy link
Member

mhkabir commented Dec 24, 2014

This is finished off. The plugin is done + tested + merged.
Just waiting for PR to be tested and merged on PX4 side.

@vooon
Copy link
Member Author

vooon commented Jan 4, 2015

PX4 support merged.

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

No branches or pull requests

3 participants