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

mjcf parser: Fix unknown size vector parsing #2535

Open
wants to merge 3 commits into
base: devel
Choose a base branch
from

Conversation

JafarAbdi
Copy link

I ran into std::__ios_failure: basic_ios::clear: iostream error errors while trying to parse g1.xml, after debugging it I found that it happens when parsing the keyframe's qpos attribute here.

This PR fixes it by changing the stream exception handling from failbit to badbit and simplifies vector parsing logic to better handle whitespace and newlines in MJCF files.

@hrp2-14
Copy link

hrp2-14 commented Dec 31, 2024

Hi ! This project doesn't usually accept pull requests on the main branch.
If this wasn't intentionnal, you can change the base branch of this PR to devel
(No need to close it for that). Best, a bot.

@JafarAbdi JafarAbdi changed the base branch from master to devel December 31, 2024 00:37
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

👋 Hi,
This is a reminder message to assign an extra build label to this Pull Request if needed.
By default, this PR will be build with minimal build options (URDF support and Python bindings)
The possible extra labels are:

  • build_collision (build Pinocchio with coal support)
  • build_casadi (build Pinocchio with CasADi support)
  • build_autodiff (build Pinocchio with CppAD support)
  • build_codegen (build Pinocchio with CppADCodeGen support)
  • build_extra (build Pinocchio with extra algorithms)
  • build_mpfr (build Pinocchio with Boost.Multiprecision support)
  • build_sdf (build Pinocchio with SDF parser)
  • build_accelerate (build Pinocchio with APPLE Accelerate framework support)
  • build_all (build Pinocchio with ALL the options stated above)

Thanks.
The Pinocchio development team.

Comment on lines +927 to +928
0.988015 0 0.154359 0
"/>
Copy link
Author

Choose a reason for hiding this comment

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

Should I revert this change? It's just meant as an extra test, otherwise a similar case is added in the test_get_unknown_size_vector_from_stream test

@jcarpent jcarpent force-pushed the fix_unknown_size_vector branch from cbfe701 to f7ca0fd Compare January 2, 2025 18:05
@jcarpent
Copy link
Contributor

jcarpent commented Jan 2, 2025

@MegMll @jorisv Could you handle this PR?

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.

3 participants