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

Added secNsecToDuration to helpers functions #158

Merged
merged 3 commits into from
Sep 16, 2020

Conversation

ahcorde
Copy link
Contributor

@ahcorde ahcorde commented Sep 14, 2020

As discussed in the PR gazebosim/gz-sensors#41 (comment) makes sense to use duration instead of time_point, This function will convert secs and nanoseconds to std::chrono::steady_clock::duration

Signed-off-by: ahcorde [email protected]

@ahcorde ahcorde added the beta Targeting beta release of upcoming collection label Sep 14, 2020
@ahcorde ahcorde self-assigned this Sep 14, 2020
@codecov
Copy link

codecov bot commented Sep 14, 2020

Codecov Report

Merging #158 into ign-math6 will decrease coverage by 0.01%.
The diff coverage is 98.03%.

Impacted file tree graph

@@              Coverage Diff              @@
##           ign-math6     #158      +/-   ##
=============================================
- Coverage      99.23%   99.22%   -0.02%     
=============================================
  Files             59       59              
  Lines           5867     5906      +39     
=============================================
+ Hits            5822     5860      +38     
- Misses            45       46       +1     
Impacted Files Coverage Δ
include/ignition/math/Helpers.hh 98.49% <98.03%> (-0.26%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bf1e3e2...7d6cdd5. Read the comment docs.

@ahcorde ahcorde changed the base branch from master to ign-math6 September 14, 2020 20:09
@scpeters
Copy link
Member

homebrew build is failing and looks like osrf/homebrew-simulation#1120

I believe this is failing now since I made swig a required dependency in osrf/homebrew-simulation#1121. I'll try removing it as a required dependency for now.

@scpeters
Copy link
Member

homebrew build is failing and looks like osrf/homebrew-simulation#1120

I believe this is failing now since I made swig a required dependency in osrf/homebrew-simulation#1121. I'll try removing it as a required dependency for now.

removing swig dependency in osrf/homebrew-simulation#1124

Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're at it, how about also adding the other functions that were added for time point? I think most of the code can be reused to avoid duplication.

  • durationToString
  • stringToDuration

include/ignition/math/Helpers.hh Show resolved Hide resolved
include/ignition/math/Helpers.hh Outdated Show resolved Hide resolved
src/Helpers_TEST.cc Outdated Show resolved Hide resolved
@chapulina chapulina added 🏰 citadel Ignition Citadel 📜 blueprint Ignition Blueprint 🔮 dome Ignition Dome labels Sep 14, 2020
@scpeters
Copy link
Member

osrf/homebrew-simulation#1124 is merged, so I expect the homebrew build to be fixed

@ahcorde
Copy link
Contributor Author

ahcorde commented Sep 16, 2020

thank you @scpeters for the fix!

Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@chapulina chapulina merged commit dbfddd4 into ign-math6 Sep 16, 2020
@chapulina chapulina deleted the ahcorde/helpers/duration branch September 16, 2020 18:28
@chapulina chapulina mentioned this pull request Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta Targeting beta release of upcoming collection 📜 blueprint Ignition Blueprint 🏰 citadel Ignition Citadel 🔮 dome Ignition Dome
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants