search

CCNYRoboticsLab / phidgets_drivers

phidgets_drivers
6 stars 15 forks source link

Add IMU diagnostics (related to #20) #24

Closed mani-monaj closed 7 years ago

mani-monaj commented 8 years ago

This is a continuation of #20. In summary, this:

  1. Moves all diagnostics logic from phidget_api to phidgets_imu
  2. Implements Periodic updates for diagnostics through processImuData()
  3. Add Frequency and Timestamp (drift) diagnostics for imu/data_raw topic
  4. Updates connect/disconnect/error related diagnostics through overridden callback functions.

I did (1) because I think phidgets_api should better remain ROS agnostic. This was also suggested by @mintar.

mintar commented 8 years ago

Thanks for your PR! Unfortunately, I'm on vacation at the moment. I'll look into this in 3 weeks.

mintar commented 8 years ago

Sorry for letting you wait for so long. I've thoroughly reviewed and tested this PR, and it looks good to me. Thanks for your work!

@muhrix : please merge.

mani-monaj commented 8 years ago

@mintar Thanks for reviewing this. I am glad this PR was helpful.

mintar commented 8 years ago

@muhrix Hello?

mintar commented 7 years ago

@muhrix Ping...

muhrix commented 7 years ago

@mintar sorry, merging now!

mintar commented 7 years ago

Great, thanks! Now I can submit two more PRs (#27 and #28).

mani-monaj commented 7 years ago

Thanks @mintar and @muhrix for merging this. Any plans for a new release of the package to indigo? Also any plans for releasing this to kinetic?

muhrix commented 7 years ago

@mani-monaj I will try and get those releases sorted soon! :-)

mintar commented 7 years ago

That would be great, I'm still waiting for that too. You remember the checklist in this comment, right?

muhrix commented 7 years ago

@mintar I do remember indeed! I had a look at it earlier today...

Before doing anything else, I think we must revisit the idea of creating phidgets_drivers somewhere else (for indigo onwards), leaving the current one here for hydro only.

@idryanov never responded, so I can only assume we can move this into ros_drivers and continue from there.

mani-monaj commented 7 years ago

@mani-monaj I will try and get those releases sorted soon! :-)

Thanks. That would be great.