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

10 ➡️ master #432

Closed
wants to merge 14 commits into from
Closed

10 ➡️ master #432

wants to merge 14 commits into from

Conversation

azeey
Copy link
Collaborator

@azeey azeey commented Dec 9, 2020

I'd appreciate some scrutiny on this PR since this is my first time doing a cherry-pick forward port on sdformat. My process was:

  • Create a branch from master called 10_master_reference
  • Merge sdf10 into 10_master_reference and fix any conflicts
  • Use git cherry -v master > cherry_list.txt and manually go through the generated text and check if those commits have been added to master. If so, remove them from the list.
  • Create another branch on master called 10_to_master
  • cat cherry_list.txt | cut -d' ' -f2 | xargs git cherry-pick and fix any conflicts
  • Compare 10_to_master and 10_master_reference and verify that the differences left over do not need to be ported.

scpeters and others added 14 commits December 8, 2020 18:15
The sdf::Console class logs some messages to a file
in `~/.sdformat/sdformat.log` by default.
This wraps the code in the constructor of the `sdf::Console`
class that opens this file in an `#ifndef` and adds
a cmake option `SDFORMAT_DISABLE_CONSOLE_LOGFILE`
to easily disable the use of this logfile.

Initial approach to gazebosim#334.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Nate Koenig <nate@openrobotics.org>

Co-authored-by: Nate Koenig <nate@openrobotics.org>
…im#245)

Signed-off-by: Silvio Traversaro <silvio.traversaro@iit.it>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>

Co-authored-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
This addresses a change in how whitespace is preserved/collapsed between
tinyxml and tinyxml2. The change primarily impacts SDF files that have
newlines, which is frequently done for aesthetics with include uris.

In this case, we use an alternate constructor for TinyXML2 that will
collapse whitespace by default.

Note that there is a performance impact of this behavior, which causes
the parsing to run essentially twice.

More information on the behavior may be found here: https://leethomason.github.io/tinyxml2/

Closes gazebosim#322

Signed-off-by: Michael Carroll <michael@openrobotics.org>

Co-authored-by: Steve Peters <scpeters@openrobotics.org>
…azebosim#357)

SDFormat has a convention of using -1 to represent positive infinity
for symmetric joint limits on velocity and effort.
If users of the DOM API are not careful and cognizant of this
convention, it can result in confusing bugs that are hard to track down.

This PR changes the API behavior so that the API returns positive
infinity instead of -1 to represent boundlessly positive values.
Hopefully this might save other SDF users from long, confusing
debugging sessions.

A test is added with an example file with default, finite and infinite
joint limit values, using both "inf" and "-1" to specify infinite values
of the effort and velocity limit.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

Co-authored-by: Michael X. Grey <grey@openrobotics.org>
These methods were deprecated in libsdformat9, so remove
them from libsdformat10.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Removed emitted error/warning when encountering a custom element & test for this

Signed-off-by: Jenn Nguyen <jenn@openrobotics.org>
* add sky dom

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* feedback changes

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* move sdf element check

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

Co-authored-by: Louise Poubel <louise@openrobotics.org>
* add double sided materail param

Signed-off-by: Ian Chen <ichen@osrfoundation.org>

* fix loading param

Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
* update changelog

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@azeey azeey requested a review from scpeters December 9, 2020 00:49
@azeey azeey self-assigned this Dec 9, 2020
@codecov-io
Copy link

Codecov Report

Merging #432 (a8bbf12) into master (983e8fa) will increase coverage by 0.21%.
The diff coverage is 99.31%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #432      +/-   ##
==========================================
+ Coverage   87.52%   87.74%   +0.21%     
==========================================
  Files          62       63       +1     
  Lines        9542     9667     +125     
==========================================
+ Hits         8352     8482     +130     
+ Misses       1190     1185       -5     
Impacted Files Coverage Δ
include/sdf/Console.hh 100.00% <ø> (ø)
src/Console.cc 100.00% <ø> (ø)
src/Filesystem.cc 98.80% <ø> (ø)
src/Utils.hh 96.66% <ø> (ø)
src/parser.cc 79.20% <94.44%> (+0.07%) ⬆️
src/JointAxis.cc 97.31% <100.00%> (+3.15%) ⬆️
src/Material.cc 98.07% <100.00%> (+0.23%) ⬆️
src/Scene.cc 100.00% <100.00%> (ø)
src/Sky.cc 100.00% <100.00%> (ø)
src/Utils.cc 100.00% <100.00%> (ø)
... and 1 more

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 983e8fa...a8bbf12. Read the comment docs.

@chapulina
Copy link
Contributor

Is this cherry-pick PR still under consideration, or will it be superseded by a merge-forward similar to #445?

@scpeters
Copy link
Member

Is this cherry-pick PR still under consideration, or will it be superseded by a merge-forward similar to #445?

I think we can replace it with a PR using a straight merge, since that will be our approach going forward

@chapulina
Copy link
Contributor

I think we can replace it with a PR using a straight merge

Sounds good. I started working on it, but there are many conflicts and I haven't been following development of this library closely. Maybe @azeey or @scpeters can open it? Otherwise, I can try to do it by comparing to this PR. Thanks!

@azeey
Copy link
Collaborator Author

azeey commented Jan 12, 2021

I will close this and open a merge forward PR.

@azeey azeey closed this Jan 12, 2021
@azeey azeey mentioned this pull request Jan 13, 2021
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.

10 participants