ouster-lidar / ouster-sdk

Ouster, Inc. sample code
Other
464 stars 438 forks source link

Reflectivity field type ([std::invalid_argument]: Accessed field at wrong type) #398

Closed vebjornjr closed 2 years ago

vebjornjr commented 2 years ago

Hello.

I have some questions regarding the field types. According to the firmware user manual, the type of Reflectivity is uint8_t (8 bit unsigned int) in both Single Return Profile and Legacy data format.

However, printing the types with:

cout << "Lidar Data Packet Format: " << to_string(info.format.udp_profile_lidar);
cout<< "\n Fields and types:";
for (const auto& [field, type] : ouster::sensor::get_format(info)) {
    cout<< "\n  " << to_string(field) << " " << to_string(type);
}

, outputs either (depending on how the sensor is configured):

 Lidar Data Packet Format: RNG19_RFL8_SIG16_NIR16
 Fields and types:
  RANGE 3
  SIGNAL 2
  REFLECTIVITY 1
  NEAR_IR 2
  FLAGS 1
  RAW32_WORD1 3
  RAW32_WORD2 3
  RAW32_WORD3 3

 Lidar Data Packet Format: LEGACY
 Fields and types:
  RANGE 3
  SIGNAL 2
  REFLECTIVITY 2
  NEAR_IR 2
  FLAGS 1
  RAW32_WORD1 3
  RAW32_WORD2 3
  RAW32_WORD3 3

By checking the ChanFieldType enum, this seems incorrect for Legacy data format (prints 2 => UINT16), while correct for RNG19_RFL8_SIG16_NIR16 format (prints 1 => UINT8).

However, accessing the field does not seems to work correctly for either of them:

const auto reflectivity = scan.field<uint8_t>(ouster::sensor::ChanField::REFLECTIVITY);  // TODO: uint16_t/uint32_t?

This throws an [std::invalid_argument]: Accessed field at wrong type exception. With the RNG19_RFL8_SIG16_NIR16 data format, I must change the type to uint16_t and with the Legacy data format I must change the type to uint32_t for it to work. This must be wrong since it does not match the user manual and does not match the printing from packet_format.

kairenw commented 2 years ago

The type of the fields in the LidarScan does not necessarily reflect those of the packet. For example, if you were to use the low bandwidth profile, the RNG15 in the profile name would not imply using a uint16_t for RANGE, as we scale the range back to 19 bits so that RANGE will always be in millimeters, and thus use a uint32_t. You can check what the expected field sizes are here: https://github.com/ouster-lidar/ouster_example/blob/master/ouster_client/src/lidar_scan.cpp#L38

For similar reasons, we've kept REFLECTIVITY at 16 bits across all non-legacy profiles since it may make it slightly easier for people who are working with REFLECTIVITY from our sensor to deal with a consistent size. [ Note for future readres: We may be changing this to 8 bits soon.]

~~In any case, I don't think you need to specify the size unless you want to. auto reflectivity = scan.field(ouster::sensor::ChanField::REFLECTIVITY) ought to work.~~ [edit: crossed out for accuracy]

vebjornjr commented 2 years ago

Oh, I understand.

However, not specifying the type does only work with Legacy data format, with the single return profile we get an [std::invalid_argument]: Accessed field at wrong type exception.

The only way we get it to work with the single return profile is by explicitly specifying the type as uint16_t, which is not ideal as we now have to change our code and recompile if we decide to change the profile.

kairenw commented 2 years ago

Which constructor are you using for LidarScan? Are you using LidarScan(w,h)[doc link]? That will default to using the field types associated with the LEGACY profile. Try the constructor LidarScan(w, h, info.format.udp_profile_lidar) instead [doc link].That should allow you to use auto and for it to populate correctly. [edit: crossed out for accuracy]

In any case: is there a reason you have to switch between LEGACY and the single returns profile? They should really be equivalent, and we don't recommend using legacy anymore generally except for recorded data where you have no choice.

vebjornjr commented 2 years ago

Here is the construction (the variables are member variables in a class):

// Configuration and calibration parameters required for parsing packet stream and calculating accurate point clouds
info = ouster::sensor::parse_metadata(metadata);

width = info.format.columns_per_frame;
height = info.format.pixels_per_column;

// Lidar data for an entire rotation
scan = ouster::LidarScan(width, height, info.format.udp_profile_lidar);

And the fetching of the reflectivities:

const auto refs = scan.field(ouster::sensor::ChanField::REFLECTIVITY);  // TODO: uint16_t/uint32_t?

As already mentioned, this works if the sensor is configured with Legacy format, but not with Single return format (must specify uint16_t).

We don't want to switch between the formats, I just did it now to test everything and find out why the types did not match as I expected.

We are stuck at the moment with using Legacy since your Ouster Studio apparently is outdated and does not support Single return profile. We use tcpdump to automatically capture interesting cases in production which we can quickly look at and analyze with Ouster Studio (which everybody with a Windows computer can use, as opposed to running custom code with visualizer).

kairenw commented 2 years ago

Okay, my apologies. I definitely lied about not having to specify the size. You're right, and it's not just with REFLECTIVITY; it's with every field since 32 is the default template parameter (you can see it in the lidar_scan.h header).

So if you're on LEGACY, you can just leave it unspecified and everything should run fine.

If however you'd like to start using the single returns profile (or low bandwidth/dual returns profiles), your Windows users could try the viz/utilities that come with that come with our Python SDK. Here it is on pypi, available for multiple platforms including Windows 10 currently. What you're looking for is probably a record utility and a viz utility. For the viz, we have simple-viz, and to record pcaps you can use this example we wrote or use it as an example to write a similar small python script that does what you need.

kairenw commented 2 years ago

Whoops. Forgot to close this out at the time. Please ping if you have more questions.