Port embedSdf script from Ruby to Python3 and provide unittests #884

Bi0T1N · 2022-03-17T16:29:48Z

🎉 New feature

Removes the Ruby dependency and thus is related to/partially solves #274 (for complete Ruby dependency removal #643 should be merged as well).

Summary

This PR converts the script that creates the .cc file for mapping of SDF filename and SDF content from Ruby to Python. Additionally a few unittests are added for the function that uses the mapping and which wasn't tested before.
For details see also the commit messages.

Note: Since I wasn't sure if this change counts as an API/ABI change or not, I targeted it to main.

Checklist

Signed all commits for DCO
Added tests
Added example and/or tutorial
Updated documentation (as needed)
Updated migration guide (as needed)
codecheck passed (See contributing)
All tests passed (See test coverage)

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

scpeters · 2022-03-17T20:38:25Z

@osrf-jenkins run tests please

sdf/embedSdf.rb

src/SDF_TEST.cc

codecov · 2022-07-01T22:49:48Z

Codecov Report

Merging #884 (7604ba4) into main (f39f571) will increase coverage by 0.01%.
The diff coverage is n/a.

❗ Current head 7604ba4 differs from pull request most recent head 2d375e3. Consider uploading reports for the commit 2d375e3 to get more accurate results

@@            Coverage Diff             @@
##             main     #884      +/-   ##
==========================================
+ Coverage   87.20%   87.21%   +0.01%     
==========================================
  Files         124      124              
  Lines       16229    16229              
==========================================
+ Hits        14152    14154       +2     
+ Misses       2077     2075       -2

Impacted Files	Coverage Δ
src/SDF.cc	`95.23% <0.00%> (+1.19%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

azeey

I left a few minor comments, but overall, this looks great! Thank you for your
contribution!

embedSdf.rb has since been updated to include a new SDF version (1.10). Do
you mind updating the python code to reflect that?

sdf/embedSdf.py

src/SDF_TEST.cc

Bi0T1N · 2022-08-26T20:13:55Z

I fixed the merge conflict that was due to the embedSdf.rb already being deleted while changes for SDF 1.10 have been added by targeting branch sdf13...it bases on sdf13 now.

this uses internal functions from GoogleTests to capture the stderr message but according to the documentation it should be fine to use them: // All macros ending with _ and symbols defined in an // internal namespace are subject to change without notice. Code // outside Google Test MUST NOT USE THEM DIRECTLY. Macros that don't // end with _ are part of Google Test's public API and can be used by // code outside Google Test. see https://github.com/google/googletest/blob/main/googletest/include/gtest/internal/gtest-port.h Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>

- removes Ruby dependency (see gazebosim#274) - improvements for cpplint checks (e.g. copyright header) Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>

it should not be operating system dependent because it is hardcoded to '/' in the C++ file that performs the lookup Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>

Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>

This reverts commit 2322458. Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>

it's the same approach as in parser_TEST.cc Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>

Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>

Bi0T1N · 2022-12-17T17:06:12Z

Apparently I misinterpreted the comments and made the changes to the wrong project. Anyway, rebased my changes on main and added Python interpreter as a build dependency with gz_build_error.

azeey · 2023-02-03T18:04:42Z

@scpeters any remaining issues?

Converts the script that creates the .cc file for mapping of SDF filename and SDF content from Ruby to Python. Additionally a few unittests are added for the function that uses the mapping and which wasn't tested before. Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com> Co-authored-by: Bi0T1N <Bi0T1N@users.noreply.github.com> Co-authored-by: Michael Carroll <michael@openrobotics.org>

These won't be used as part of the cmake build on this branch, but are useful for generating the same file from bazel. Co-authored-by: Bi0T1N <Bi0T1N@users.noreply.github.com> Co-authored-by: Michael Carroll <michael@openrobotics.org> Signed-off-by: Michael Carroll <michael@openrobotics.org>

* Cherry-pick the python3 embedSdf script and tests (#884) These won't be used as part of the cmake build on this branch, but are useful for generating the same file from bazel. Co-authored-by: Bi0T1N <Bi0T1N@users.noreply.github.com> Co-authored-by: Michael Carroll <michael@openrobotics.org> Signed-off-by: Michael Carroll <michael@openrobotics.org> * Improvements to embedSdf script Signed-off-by: Michael Carroll <michael@openrobotics.org> * Update sdf/CMakeLists.txt Signed-off-by: Michael Carroll <michael@openrobotics.org> Co-authored-by: Addisu Z. Taddese <addisu@openrobotics.org> * Update bazel files Signed-off-by: Michael Carroll <michael@openrobotics.org> * Lint Signed-off-by: Michael Carroll <michael@openrobotics.org> * Add utils back to parser Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai> * Fix gz_TEST Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai> * Ignore pycache folders Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai> * Fix parser_TEST Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai> * Bazel nits Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai> * Fix path logic Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai> * Fix visibility Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai> * Fix strings Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai> * Use standard vars Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai> * Fix string conversions Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai> * Fix string conversions Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai> * Fix embedded sdf Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai> * Fix converter test Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai> * Lint Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai> * Update bazel files Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai> * Add detail prefix Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai> * Fix test path Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai> * Set homepath on Windows Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai> --------- Signed-off-by: Michael Carroll <michael@openrobotics.org> Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai> Co-authored-by: Bi0T1N <28802083+Bi0T1N@users.noreply.github.com> Co-authored-by: Bi0T1N <Bi0T1N@users.noreply.github.com> Co-authored-by: Addisu Z. Taddese <addisu@openrobotics.org>

Bi0T1N requested review from azeey and scpeters as code owners March 17, 2022 16:29

github-actions bot added the 🌱 garden Ignition Garden label Mar 17, 2022

scpeters reviewed Mar 17, 2022

View reviewed changes

sdf/embedSdf.rb Show resolved Hide resolved

Bi0T1N force-pushed the embeddedsdf_refactoring branch from 2fb48f1 to df97b05 Compare March 18, 2022 16:57

scpeters reviewed Mar 18, 2022

View reviewed changes

src/SDF_TEST.cc Outdated Show resolved Hide resolved

Bi0T1N force-pushed the embeddedsdf_refactoring branch 2 times, most recently from 625bc4d to 934cac5 Compare March 18, 2022 21:27

chapulina added the needs upstream release Blocked by a release of an upstream library label Apr 4, 2022

chapulina requested a review from mjcarroll May 2, 2022 18:54

Bi0T1N force-pushed the embeddedsdf_refactoring branch from 55a990a to 0862e66 Compare July 1, 2022 20:34

Bi0T1N requested a review from scpeters July 2, 2022 14:00

chapulina added the bug Something isn't working label Jul 23, 2022

azeey reviewed Aug 5, 2022

View reviewed changes

sdf/embedSdf.py Outdated Show resolved Hide resolved

sdf/embedSdf.py Outdated Show resolved Hide resolved

sdf/embedSdf.py Outdated Show resolved Hide resolved

src/SDF_TEST.cc Outdated Show resolved Hide resolved

chapulina changed the base branch from main to sdf13 August 5, 2022 19:05

Bi0T1N force-pushed the embeddedsdf_refactoring branch from 939219a to 7395479 Compare August 20, 2022 21:00

Bi0T1N requested review from azeey and removed request for mjcarroll and scpeters August 20, 2022 21:17

Bi0T1N force-pushed the embeddedsdf_refactoring branch from 7395479 to e8f56ba Compare August 26, 2022 20:07

chapulina removed the needs upstream release Blocked by a release of an upstream library label Aug 26, 2022

Bi0T1N added 13 commits December 17, 2022 17:27

Port embedSdf script from Ruby to Python3

3901028

- removes Ruby dependency (see gazebosim#274) - improvements for cpplint checks (e.g. copyright header) Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>

Hardcode separator in generated C++ mapping to '/'

f7c502c

it should not be operating system dependent because it is hardcoded to '/' in the C++ file that performs the lookup Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>

Output variable to trace Windows issue

59fb656

Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>

Revert "Output variable to trace Windows issue"

30fbd1d

This reverts commit 2322458. Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>

Refactor code to avoid GTest GetCapturedStderr

f10c255

it's the same approach as in parser_TEST.cc Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>

Use Python3_EXECUTABLE variable for command execution

42c1cb0

Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>

Fix line length for cpplint

468e0c0

Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>

Use multiline strings in combination with cleandoc

37a4054

Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>

Use function argument variable

26fcd31

Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>

Add changes for new SDF version 1.10

25adf79

Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>

Use testing::RedirectConsoleStream to capture errors

faf92e4

Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>

Make Python a build dependency

bbe6714

Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>

Bi0T1N force-pushed the embeddedsdf_refactoring branch from e16e985 to bbe6714 Compare December 17, 2022 17:01

Bi0T1N requested a review from scpeters December 17, 2022 17:08

Merge branch 'main' into embeddedsdf_refactoring

2d375e3

mjcarroll approved these changes Feb 2, 2023

View reviewed changes

azeey requested a review from mjcarroll February 3, 2023 18:03

scpeters approved these changes Feb 6, 2023

View reviewed changes

azeey merged commit 0642526 into gazebosim:main Feb 7, 2023

mjcarroll mentioned this pull request Feb 8, 2023

Python script to replace ruby xmlschema generator #1232

Merged

Bi0T1N deleted the embeddedsdf_refactoring branch February 10, 2023 16:09

mjcarroll mentioned this pull request Feb 16, 2023

Backport the python3 embedSdf script variant #1240

Merged

scpeters mentioned this pull request Sep 27, 2023

Prepare for 14.0.0~pre3 Release #1331

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port embedSdf script from Ruby to Python3 and provide unittests #884

Port embedSdf script from Ruby to Python3 and provide unittests #884

Bi0T1N commented Mar 17, 2022 •

edited

Loading

scpeters commented Mar 17, 2022

codecov bot commented Jul 1, 2022 •

edited

Loading

azeey left a comment

Bi0T1N commented Aug 26, 2022 •

edited

Loading

Bi0T1N commented Dec 17, 2022

azeey commented Feb 3, 2023

Port embedSdf script from Ruby to Python3 and provide unittests #884

Port embedSdf script from Ruby to Python3 and provide unittests #884

Conversation

Bi0T1N commented Mar 17, 2022 • edited Loading

🎉 New feature

Summary

Checklist

scpeters commented Mar 17, 2022

codecov bot commented Jul 1, 2022 • edited Loading

Codecov Report

azeey left a comment

Choose a reason for hiding this comment

Bi0T1N commented Aug 26, 2022 • edited Loading

Bi0T1N commented Dec 17, 2022

azeey commented Feb 3, 2023

Bi0T1N commented Mar 17, 2022 •

edited

Loading

codecov bot commented Jul 1, 2022 •

edited

Loading

Bi0T1N commented Aug 26, 2022 •

edited

Loading