Skip to content

Add string to time function#152

Merged
chapulina merged 33 commits intoign-math6from
jshep1/string_to_chrono_helper
Sep 4, 2020
Merged

Add string to time function#152
chapulina merged 33 commits intoign-math6from
jshep1/string_to_chrono_helper

Conversation

@JShep1
Copy link
Copy Markdown

@JShep1 JShep1 commented Aug 28, 2020

Adds a function converting a string of the general format "dd hh:mm:ss.nnn" where n is millisecond values to a std::chrono::steady_clock::time_point

Depends on #150

Signed-off-by: John Shepherd john@openrobotics.org

ahcorde and others added 9 commits August 26, 2020 11:17
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: John Shepherd <john@openrobotics.org>
@JShep1 JShep1 requested a review from ahcorde August 28, 2020 04:36
@JShep1 JShep1 requested a review from scpeters as a code owner August 28, 2020 04:36
John Shepherd added 3 commits August 27, 2020 21:39
Signed-off-by: John Shepherd <john@openrobotics.org>
…gnitionrobotics/ign-math into jshep1/string_to_chrono_helper

Signed-off-by: John Shepherd <john@openrobotics.org>
Signed-off-by: John Shepherd <john@openrobotics.org>
@JShep1 JShep1 added 📜 blueprint Ignition Blueprint enhancement New feature or request labels Aug 28, 2020
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 28, 2020

Codecov Report

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

Impacted file tree graph

@@              Coverage Diff              @@
##           ign-math6     #152      +/-   ##
=============================================
- Coverage      99.24%   99.23%   -0.02%     
=============================================
  Files             59       59              
  Lines           5823     5867      +44     
=============================================
+ Hits            5779     5822      +43     
- Misses            44       45       +1     
Impacted Files Coverage Δ
include/ignition/math/Helpers.hh 98.75% <97.72%> (-0.39%) ⬇️

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 d5dc70f...65d145e. Read the comment docs.

John Shepherd and others added 3 commits August 27, 2020 22:49
Signed-off-by: John Shepherd <john@openrobotics.org>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Copy link
Copy Markdown
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

Just a comment about try/catch

John Shepherd and others added 3 commits August 28, 2020 10:23
Signed-off-by: John Shepherd <john@openrobotics.org>
Signed-off-by: John Shepherd <john@openrobotics.org>
Cast to system_clock::duration where necessary.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Copy link
Copy Markdown
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

John Shepherd and others added 6 commits August 31, 2020 10:11
Signed-off-by: John Shepherd <john@openrobotics.org>
…rono_helper

Signed-off-by: John Shepherd <john@openrobotics.org>
Signed-off-by: John Shepherd <john@openrobotics.org>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
…rono_helper

Signed-off-by: John Shepherd <john@openrobotics.org>
Signed-off-by: John Shepherd <john@openrobotics.org>
@JShep1 JShep1 requested a review from ahcorde September 2, 2020 00:28
Signed-off-by: ahcorde <ahcorde@gmail.com>
Copy link
Copy Markdown
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

LGTM

ahcorde and others added 2 commits September 2, 2020 17:57
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: John Shepherd <john@openrobotics.org>
@chapulina chapulina added 🏰 citadel Ignition Citadel 🔮 dome Ignition Dome labels Sep 2, 2020
Base automatically changed from ahcorde/time/helper_functions to ign-math6 September 3, 2020 07:47
Signed-off-by: John Shepherd <john@openrobotics.org>
Signed-off-by: John Shepherd <john@openrobotics.org>
@chapulina
Copy link
Copy Markdown
Contributor

Not sure why codecov is marking some lines as uncovered, when they clearly are:

https://codecov.io/gh/ignitionrobotics/ign-math/pull/152/src/include/ignition/math/Helpers.hh?before=include/ignition/math/Helpers.hh

@JShep1
Copy link
Copy Markdown
Author

JShep1 commented Sep 3, 2020

Not sure why codecov is marking some lines as uncovered, when they clearly are:

https://codecov.io/gh/ignitionrobotics/ign-math/pull/152/src/include/ignition/math/Helpers.hh?before=include/ignition/math/Helpers.hh

I was also wondering this

@chapulina chapulina merged commit 87d3406 into ign-math6 Sep 4, 2020
@chapulina chapulina deleted the jshep1/string_to_chrono_helper branch September 4, 2020 20:37
@scpeters scpeters mentioned this pull request Sep 4, 2020
@scpeters
Copy link
Copy Markdown
Member

scpeters commented Sep 4, 2020

sdformat is building ign-math6 from source and is now reporting two windows warnings from code added in this PR:

const std::string &_timeString)
{
std::chrono::steady_clock::time_point timePoint =
math::secNsecToTimePoint(-1, 0);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm seeing an msbuild warning when compiling this from source for sdformat:

'argument': conversion from 'int' to 'const uint64_t', signed/unsigned mismatch

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm guessing it's complaining about the implicit cast of -1 to const uint64_t

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes, I just realized this, I guess an invalid string will have to return 0 now, I was using -1 as an indication of error, but now it seems that's not going to be possible

{
numberDays = std::stoi(dayString);
}
catch (const std::out_of_range &oor)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

another windows warning found by sdformat:

'oor': unreferenced local variable

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this would be easy enough to fix by just deleting oor:

catch (const std::out_of_range &)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for pointing this out, I'm creating a PR now

@JShep1 JShep1 mentioned this pull request Sep 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📜 blueprint Ignition Blueprint 🏰 citadel Ignition Citadel 🔮 dome Ignition Dome enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants