ouster-lidar / ouster-sdk

Ouster, Inc. sample code
Other
458 stars 437 forks source link

Incorrect configuration using a sensor_config object in C++ #586

Closed laurent82 closed 4 months ago

laurent82 commented 5 months ago

Describe the bug The sensor_config object has unexpected behavior and prevents the correct configuration of the sensor. + Memory crash when the ouster::sensor::sensor_config object is deleted (at the end of the function). To Reproduce

Steps to reproduce the behavior (steps below are just an example): Create a new sensor_config object

ouster::sensor::sensor_config sensorConfig;
sensorConfig.operating_mode = ouster::sensor::OperatingMode::OPERATING_NORMAL;
sensorConfig.udp_dest = "";
sensorConfig.ld_mode = ouster::sensor::lidar_mode::MODE_1024x10;
sensorConfig.azimuth_window = std::make_pair(0, 180000);
sensorConfig.udp_port_lidar = 7502;
sensorConfig.udp_port_imu = 7503;
sensorConfig.signal_multiplier = 1;
 sensorConfig.udp_profile_lidar = ouster::sensor::UDPProfileLidar::PROFILE_RNG19_RFL8_SIG16_NIR16;
 sensorConfig.ts_mode = ouster::sensor::timestamp_mode::TIME_FROM_SYNC_PULSE_IN;
 sensorConfig.sync_pulse_in_polarity = ouster::sensor::POLARITY_ACTIVE_HIGH;
 sensorConfig.nmea_in_polarity = ouster::sensor::POLARITY_ACTIVE_HIGH;
 sensorConfig.nmea_baud_rate = ouster::sensor::NMEABaudRate::BAUD_115200;
 sensorConfig.nmea_leap_seconds = 0;
 sensorConfig.multipurpose_io_mode = ouster::sensor::MultipurposeIOMode::MULTIPURPOSE_INPUT_NMEA_UART;
 sensorConfig.sync_pulse_out_polarity = ouster::sensor::POLARITY_ACTIVE_HIGH;
 sensorConfig.sync_pulse_out_frequency = 1;
 sensorConfig.sync_pulse_out_angle = 360;
 sensorConfig.sync_pulse_out_pulse_width = 10;
 sensorConfig.phase_lock_enable = false;
 sensorConfig.udp_profile_imu = ouster::sensor::UDPProfileIMU::PROFILE_IMU_LEGACY;
 sensorConfig.columns_per_packet = 16;

Now just print the result using cout << ouster::sensor::to_string(sensorConfig); and there is the result:

{
    "columns_per_packet": 1,
    "lidar_mode": "512x10",
    "multipurpose_io_mode": "OFF",
    "nmea_baud_rate": "BAUD_9600",
    "nmea_in_polarity": "ACTIVE_LOW",
    "operating_mode": "NORMAL",
    "phase_lock_offset": 0,
    "sync_pulse_in_polarity": "ACTIVE_LOW",
    "sync_pulse_out_angle": 1,
    "sync_pulse_out_frequency": 1,
    "sync_pulse_out_polarity": "ACTIVE_LOW",
    "sync_pulse_out_pulse_width": 1,
    "timestamp_mode": "TIME_FROM_INTERNAL_OSC",
    "udp_port_imu": 1,
    "udp_port_lidar": 1,
    "udp_profile_imu": "LEGACY",
    "udp_profile_lidar": "LEGACY"
}

This is not the expected result.

Now if I create a Json::Value object and convert it to a sensor_config object, the values are the good ones, except the azimuth window.

 Json::Value jsonConfig = config_to_json(sensorConfig);
 sensorConfig = ouster::sensor::parse_config(jsonConfig.toStyledString());
{
    "azimuth_window":
    [
        0,
        1
    ],
    "columns_per_packet": 16,
    "lidar_mode": "1024x10",
    "multipurpose_io_mode": "INPUT_NMEA_UART",
    "nmea_baud_rate": "BAUD_115200",
    "nmea_in_polarity": "ACTIVE_HIGH",
    "nmea_leap_seconds": 0,
    "operating_mode": "NORMAL",
    "phase_lock_enable": false,
    "signal_multiplier": 1,
    "sync_pulse_in_polarity": "ACTIVE_HIGH",
    "sync_pulse_out_angle": 360,
    "sync_pulse_out_frequency": 1,
    "sync_pulse_out_polarity": "ACTIVE_HIGH",
    "sync_pulse_out_pulse_width": 10,
    "timestamp_mode": "TIME_FROM_SYNC_PULSE_IN",
    "udp_dest": "",
    "udp_port_imu": 7503,
    "udp_port_lidar": 7502,
    "udp_profile_imu": "LEGACY",
    "udp_profile_lidar": "RNG19_RFL8_SIG16_NIR16"
}

When end of the function is reached, and thus the sensor_config is automatically deleted, the application crashs (exited with code -1073741819)

Platform (please complete the following information):

twslankard commented 5 months ago

Thanks for the report, @laurent82.

Unfortunately, I'm not able to replicate the issue. Can you please tell me which commit you're using of the ouster_example repo, and whether you have any local modifications?

Cheers, Tom

Staff Engineer Ouster Inc.

laurent82 commented 5 months ago

Hi, I'm using the last commit (of Ouster SDK) without any modification. I can reproduce this on both Windows (x64) and Ubuntu (aarch64). If it can help, I have this CMake warning:

[cmake] -- Found OusterSDK: C:/xenolabs/projects/CPP_Michelin_Recorder/external/lib/ouster/lib/cmake/OusterSDK/OusterSDKConfig.cmake
[cmake] CMake Deprecation Warning at external/vcpkg/installed/x64-windows/share/jsoncpp/jsoncppConfig.cmake:2 (cmake_policy):
[cmake]   Compatibility with CMake < 3.5 will be removed from a future version of
[cmake]   CMake.
[cmake] 
[cmake]   Update the VERSION argument <min> value or use a ...<max> suffix to tell
[cmake]   CMake that the project does not need compatibility with older versions

And to build the OusterSDK, I followed the installation guide. For this : PS > .\vcpkg.exe install --triplet x64-windows jsoncpp eigen3 curl libtins glfw3 glew spdlog libpng flatbuffers I have initially used the 2023.02.24 tag but I had to update vcpkg otherwise zlib failed to build.

mrizzwan commented 4 months ago

I am getting the same issue while trying to delete the object of the class containing osuter::sensor::sensor_info as member, sensor::info contains sensor_config as member, when monitored through debugger, back-trace gives this output when crash occurs:


[Thread 0x7fff8b7fe640 (LWP 144007) exited]
dataThread stopped
[Thread 0x7fff8bfff640 (LWP 144005) exited]
reset slot for lidarWriter called
lidar_writer destructor
reset slot for lidarData called
lidar_data destructor

Thread 1 "app" received signal SIGSEGV, Segmentation fault.
0x00007ffff1ca53fe in __GI___libc_free (mem=0x1) at ./malloc/malloc.c:3368
3368    ./malloc/malloc.c: No such file or directory.
(gdb) bt
#0  0x00007ffff1ca53fe in __GI___libc_free (mem=0x1) at ./malloc/malloc.c:3368
#1  0x00005555555a1f3a in std::_Optional_payload_base<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >::_M_destroy() (this=0x555556b211f0) at /usr/include/c++/11/optional:260
#2  0x00005555555a0686 in std::_Optional_payload_base<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >::_M_reset() (this=0x555556b211f0) at /usr/include/c++/11/optional:280
#3  0x000055555559f1e0 in std::_Optional_payload<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, false, false, false>::~_Optional_payload()
    (this=0x555556b211f0, __in_chrg=<optimized out>) at /usr/include/c++/11/optional:401
#4  0x000055555559dc32 in std::_Optional_base<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, false, false>::~_Optional_base() (this=0x555556b211f0, __in_chrg=<optimized out>)
    at /usr/include/c++/11/optional:472
#5  0x000055555559e4cc in std::optional<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >::~optional() (this=0x555556b211f0, __in_chrg=<optimized out>)
    at /usr/include/c++/11/optional:662
#6  0x000055555559e510 in ouster::sensor::sensor_config::~sensor_config()
    (this=0x555556b211f0, __in_chrg=<optimized out>) at /usr/local/include/ouster/types.h:233
#7  0x000055555559e548 in ouster::sensor::sensor_info::~sensor_info()
    (this=0x555556b20e38, __in_chrg=<optimized out>) at /usr/local/include/ouster/types.h:385
#8  0x000055555559ade4 in cLidarHandler::~cLidarHandler() (this=0x555556b20e00, __in_chrg=<optimized out>)```
twslankard commented 4 months ago

Thanks for the additional report @mrizzwan.

I am still not able to reproduce the either issue on Linux AMD 64 systems. In all cases, I am seeing the correct output from to_string and not encountering a crash from the sensor_config destructor.

We're still looking into this. However, to speed things up would either of you be able to share a complete example of code that causes either problem?

Samahu commented 4 months ago

@laurent82 we suspect the issue is related to this issue and this one in which your application is linking to two different optional versions. Please checkout the response on the issues that I linked.

twslankard commented 4 months ago

@laurent82 @mrizzwan

I'm able to reproduce both issues when using C++14 for building ouster-sdk but C++20 for building my application.

Try specifying -DCMAKE_CXX_STANDARD=20 both for when you are building the Ouster SDK libs and your project if you haven't already. When I do this, both the incorrect values and the segmentation faults go away.

mrizzwan commented 4 months ago

Yes, my problem is resolved. My application was using C++17, so i modified the ouster_example's CMakeLists.txt file to set set(CMAKE_CXX_STANDARD 17), then rebuild and reinstalled it. In this way, both were using C++17, and no crash was observed and object deletion was done successfully.

Thanks for the guidance!

laurent82 commented 4 months ago

Yes, new version (0.11.0 and using -DCMAKE_CXX_STANDARD=17 seem to fix the problem!

Unfortunatly, Ouster SDK does not build out-of-the-box with -DCMAKE_CXX_STANDARD=20 I have the following error (both Windows&Linux): \ouster_client\src\curl_client.h(93,54): error C7595: 'fmt::v10::basic_format_string<char,const std::string &,long &,std::string &>::basic_format_string': call to immediate function is not a constant expression

I will keep on C++17 for the moment hoping this will be fixed. Thank you!

mrizzwan commented 4 months ago

for fixing the error for -DCMAKE_CXX_STANDARD=20: got to line#91 in the file \ouster_client\src\curl_client.h, replace the following code with a simple string literal instead of std::string(""):

ouster::sensor::logger().warn(
                    std::string("Re-attempting CurlClient::execute_get after "
                                "failure for url: ") +
                        "[{}] with the code: [{}] - and return: {}",
                    url, http_code, buffer);
laurent82 commented 4 months ago

You, sir, certainly made my day!

twslankard commented 4 months ago

Great! I'm glad to hear the issues are resolved. Thank you both for your patience. Please don't hesitate to reach out again if you encounter other issues or questions.

Cheers, Tom