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

FW logs parser should parse :f format as 32 bit float #12652

Merged
merged 2 commits into from
Feb 12, 2024

Conversation

OhadMeir
Copy link
Contributor

Current parser have no difference between {0} and {0:f} formats in the xml

@OhadMeir OhadMeir requested a review from Nir-Az February 11, 2024 13:40
@@ -56,7 +56,7 @@ namespace librealsense

st_regular_exp[2] << "\\{\\b(" << i << "):f\\}";
regular_exp[2] = st_regular_exp[2].str();
st_replacement[2] << params[i];
st_replacement[2] << *reinterpret_cast< const float * >( &params[i] ); // Parse as 4 raw bytes of float
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add some std check of is_floating_point?
https://en.cppreference.com/w/cpp/types/is_floating_point

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think is_floating_point will help in this case. std::isfinite is more appropriate here (https://en.cppreference.com/w/cpp/numeric/math/isfinite).
If you want I will add a check. If it is not finite, should I print as decimal int, hexadecimal int or throw?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure what a thrown will cause here, but maybe we can LOG_ERROR and print as HEX?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@OhadMeir OhadMeir merged commit faaab32 into IntelRealSense:development Feb 12, 2024
16 of 17 checks passed
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.

2 participants