Skip to content

Gyro sensitivity feature - #12715

Merged
Nir-Az merged 21 commits into
realsenseai:developmentfrom
noacoohen:IMU_sensitivity
Mar 17, 2024
Merged

Gyro sensitivity feature#12715
Nir-Az merged 21 commits into
realsenseai:developmentfrom
noacoohen:IMU_sensitivity

Conversation

@noacoohen

Copy link
Copy Markdown
Contributor

Added gyro sensitivity feature for windows+Linux for D435i (Old/New sensor) and D455

tracked by RSDSO-19305

@noacoohen
noacoohen requested a review from Nir-Az February 29, 2024 19:53
Comment thread include/librealsense2/h/rs_option.h Outdated
Comment thread src/ds/d400/d400-factory.cpp Outdated
Comment thread src/ds/d400/d400-factory.cpp Outdated
Comment thread src/ds/d400/d400-motion.cpp Outdated
Comment thread src/ds/d400/d400-motion.cpp
Comment thread src/ds/d400/d400-options.cpp Outdated
Comment thread src/ds/d400/d400-options.cpp Outdated
if( auto strong = _sensor.lock() )
return strong->get_imu_sensitivity( RS2_STREAM_GYRO );
else
return -1;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why not throw if we cannot query it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment thread src/ds/d400/d400-options.cpp Outdated
{
switch( static_cast< int >( val ) )
{
case 0: {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

So I understand the user input is in range of 0-4?
So can we create an Enum for it instead of this description?
And we need somewhere to place the units
It can be
RS_OPTION_GYRO_SENSITIVITY_61_0_DEG_SEC?
Let's speak about it on Sunday

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment thread src/ds/d400/d400-options.h Outdated

#include <rsutils/lazy.h>

#include <src/hid-sensor.h>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can be removed from the header as we use only pointers here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Comment thread src/ds/d400/d400-options.h
@@ -0,0 +1,29 @@
// License: Apache 2.0. See LICENSE file in root directory.
// Copyright(c) 2023 Intel Corporation. All Rights Reserved.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -0,0 +1,29 @@
// License: Apache 2.0. See LICENSE file in root directory.
// Copyright(c) 2023 Intel Corporation. All Rights Reserved.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment thread src/ds/features/gyro-sensitivity-feature.h
Comment thread src/hid-sensor.cpp Outdated
static const std::map< float, double > gyro_sensitivity_convert
= { { 0.0f, 0},
{ 1.0f, 0.1},
{ 2.0f, 0.2},

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

space alignment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment thread src/hid-sensor.cpp Outdated
if( _imu_sensitivity_per_rs2_stream.find( stream ) != _imu_sensitivity_per_rs2_stream.end() )
{
return _imu_sensitivity_per_rs2_stream[stream];
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

format alignment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment thread src/hid-sensor.cpp Outdated
Comment thread src/hid-sensor.cpp
Comment thread src/hid-sensor.cpp Outdated
Comment thread src/hid-sensor.cpp
Comment thread src/hid-sensor.h Outdated
Comment thread src/linux/backend-hid.cpp
Comment thread src/linux/backend-hid.cpp
Comment thread src/linux/backend-hid.cpp
Comment thread src/proc/motion-transform.cpp Outdated
{
// Default sensitivity is +-2000 deg/sec at 16.384 LSB/Deg/Sec (LSB is 0.1220703125 deg/sec, historically rounded to 0.1).
// High sensitivity is +-125 deg/sec at 262.144 LSB/Deg/Sec (LSB is 0.003814697265625 deg/sec).
// Default sensitivity in HKR is +-2000 deg/sec at 16.384 LSB/Deg/Sec.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let's please remove HKR word, this comment was about d400.
but FW say it was +-1000 and not +-2000

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment thread src/ds/d400/d400-factory.cpp Outdated
dev_info, d400_device::_hw_monitor, get_firmware_logs_command(), get_flash_logs_command() )
{
check_and_restore_rgb_stream_extrinsic();
if( _fw_version >= firmware_version( 5, 15, 1, 224 ) )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this the formal FW version or just a temporary build?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This should change to 5.16.0.0 before we merge it

Comment thread src/ds/features/gyro-sensitivity-feature.cpp Outdated
Comment thread src/ds/d400/d400-options.cpp Outdated
Comment thread src/ds/d400/d400-options.cpp Outdated
Comment thread src/ds/ds-motion-common.cpp Outdated
Comment thread src/platform/hid-data.h Outdated
Comment thread src/platform/hid-device.h Outdated
Comment thread src/mf/mf-hid.h
Comment thread src/mf/mf-hid.cpp
Comment thread src/to-string.cpp
#define CASE( X ) STRCASE( GYRO_SENSITIVITY, X )
switch( value )
{
CASE( 61_0_MILLI_DEG_SEC )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please add a unit test setting the option from python, to validate the option works

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Next PR :)

Comment thread src/realsense.def
Comment thread src/hid-sensor.h Outdated
Comment thread src/hid-sensor.cpp Outdated
Comment thread src/ds/d400/d400-motion.cpp Outdated
Comment thread src/ds/ds-motion-common.cpp

@Nir-Az Nir-Az left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Great work!

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