Skip to content

Conversation

@aangerma
Copy link
Contributor

Fix for RS5-8691

{
bool ret = false;
auto motion = frame.as< motion_frame >();
if( frame )
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. if (motionframe)
  2. the two lines can be merged into:
  if (auto motion = frame.as< motion_frame >())
  {
    auto axes = motion.get_motion_data();
    ...

{
bool ret = false;
auto pose = frame.as< pose_frame >();
if( frame )
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

csv << "Timestamp (ms)," << std::fixed << std::setprecision( 2 )
<< frame.get_timestamp() << std::endl;
csv << std::setprecision( 7 ) << "acceleration," << pose_data.acceleration << std::endl;
csv << std::setprecision( 7 ) << "angular_acceleration," << pose_data.angular_acceleration << std::endl;
Copy link
Contributor

Choose a reason for hiding this comment

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

Each one of those is a vector-like structure - need to verify that they will be presented comma-separated x,y,z,w.
Specify units at the beginning/end of each field

Please attach an actual pose CSV captured for motion and pose for review

csv << "Frame Number," << frame.get_frame_number() << std::endl;
csv << "Timestamp (ms)," << std::fixed << std::setprecision( 2 )
<< frame.get_timestamp() << std::endl;
csv << std::setprecision( 7 ) << "x," << axes.x << std::endl;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please put those three values in a single CSV line + add units

}

auto pose = last_frame.as< pose_frame >();
if (pose && pose.get_profile().stream_type() == RS2_STREAM_POSE)
Copy link
Contributor

Choose a reason for hiding this comment

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

The stream type check can be omitted - imo redundant


auto motion = last_frame.as< motion_frame >();
if( motion
&& val_in_range( motion.get_profile().stream_type(),
Copy link
Contributor

Choose a reason for hiding this comment

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

stream check can be omitted

ss << "The frame attributes are saved into\n" << filename;
else
viewer.not_model->add_notification(
{ to_string() << "Failed to save frame metadata file " << filename,
Copy link
Contributor

Choose a reason for hiding this comment

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

Failed to save TBD frame metadata file

ss << "The frame attributes are saved into\n" << filename;
else
viewer.not_model->add_notification(
{ to_string() << "Failed to save frame metadata file " << filename,
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

Copy link
Contributor

@ev-mp ev-mp left a comment

Choose a reason for hiding this comment

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

Looks good

@ev-mp ev-mp merged commit 5f5a7a3 into realsenseai:development Oct 5, 2020
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