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

Add IMU diagnostics (related to #20) #24

Merged
merged 3 commits into from
Nov 20, 2016

Conversation

mani-monaj
Copy link
Contributor

@mani-monaj mani-monaj commented Jul 15, 2016

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.

keshaviyengar and others added 3 commits July 14, 2016 14:38
* Added "/" for closing tags in package.xml
* Moved lines to non-static function as required.
- Remove all diagnostics logic from phidget_api
- Add diagnostics to phidgets_imu
  - Connect/disconnect/error states are propogated from callbacks
  - Periodic updates for diagnostics through processImuData()
  - Add Frequency and Timestamp (drift) diagnostics for imu/data_raw
  topic
@mintar
Copy link
Contributor

mintar commented Jul 15, 2016

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

@mintar
Copy link
Contributor

mintar commented Sep 6, 2016

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
Copy link
Contributor Author

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

@mintar
Copy link
Contributor

mintar commented Sep 12, 2016

@muhrix Hello?

@mintar
Copy link
Contributor

mintar commented Nov 19, 2016

@muhrix Ping...

@muhrix
Copy link
Collaborator

muhrix commented Nov 20, 2016

@mintar sorry, merging now!

@muhrix muhrix merged commit 809f1bd into CCNYRoboticsLab:master Nov 20, 2016
@mintar
Copy link
Contributor

mintar commented Nov 21, 2016

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

@mani-monaj
Copy link
Contributor Author

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
Copy link
Collaborator

muhrix commented Nov 22, 2016

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

@mintar
Copy link
Contributor

mintar commented Nov 22, 2016

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

@muhrix
Copy link
Collaborator

muhrix commented Nov 22, 2016

@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 mani-monaj deleted the imu-diag branch November 22, 2016 17:48
@mani-monaj
Copy link
Contributor Author

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

Thanks. That would be great.

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

Successfully merging this pull request may close these issues.

4 participants