Skip to content

Comments

[RSDK-12899] Fix set device properties#64

Open
SebastianMunozP wants to merge 8 commits intoviam-modules:mainfrom
SebastianMunozP:fix-get-device-properties
Open

[RSDK-12899] Fix set device properties#64
SebastianMunozP wants to merge 8 commits intoviam-modules:mainfrom
SebastianMunozP:fix-get-device-properties

Conversation

@SebastianMunozP
Copy link
Contributor

@SebastianMunozP SebastianMunozP commented Feb 3, 2026

This PR fixes a crash on the get_device_properties do_command, caused by a missing continue statement on get_device_properties handling.

This issue happens when the user calls get_device_properties with a non-struct payload, such as this:

{
  "set_device_properties": ""
}

With this fix, these kind of calls are correctly handled producing the following error

{
  "error": "calling set_device_properties on non-struct properties"
}

We are also checking that all settings being modified can actually be modified before doing it

This works OK:

{
  "set_device_properties":   
  {"OB_PROP_DEPTH_NOISE_REMOVAL_FILTER_MAX_SPECKLE_SIZE_INT": {
    "name": "OB_PROP_DEPTH_NOISE_REMOVAL_FILTER_MAX_SPECKLE_SIZE_INT",
    "type": "int",
    "max": 1000,
    "current": 1000,
    "id": 41,
    "default": 1000,
    "min": 1,
    "permission": "read_write"
  },
  "OB_PROP_DEPTH_ALIGN_HARDWARE_MODE_INT": {
    "current": 2,
    "min": 0,
    "max": 5,
    "permission": "read_write",
    "default": 0,
    "name": "OB_PROP_DEPTH_ALIGN_HARDWARE_MODE_INT",
    "id": 63,
    "type": "int"
  }
  }

This does not (missing current field), no fields are modified

{
  "set_device_properties":   
  {"OB_PROP_DEPTH_NOISE_REMOVAL_FILTER_MAX_SPECKLE_SIZE_INT": {
    "name": "OB_PROP_DEPTH_NOISE_REMOVAL_FILTER_MAX_SPECKLE_SIZE_INT",
    "type": "int",
    "max": 1000,    "id": 41,
    "default": 1000,
    "min": 1,
    "permission": "read_write"
  },
  "OB_PROP_DEPTH_ALIGN_HARDWARE_MODE_INT": {
    "current": 5,
    "min": 0,
    "max": 5,
    "permission": "read_write",
    "default": 0,
    "name": "OB_PROP_DEPTH_ALIGN_HARDWARE_MODE_INT",
    "id": 63,
    "type": "int"
  }
  }
}

@SebastianMunozP SebastianMunozP changed the title Fix get device properties [RSDK-12899] Fix get device properties Feb 3, 2026
@hexbabe
Copy link
Collaborator

hexbabe commented Feb 3, 2026

PR title is inaccurate. This fix is for set device properties

Copy link
Collaborator

Choose a reason for hiding this comment

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

this is dead code btw

Choose a reason for hiding this comment

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

Do we want to skip with continue or error out here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, changing this to error out, also, since this is intended to modify multiple settings of the camera at the same time, I'm adding a 2 phase approach, in which first we check if all changes can be applied, and only when we are sure, we apply them,

Copy link

@seanavery seanavery left a comment

Choose a reason for hiding this comment

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

LGTM!

Do we want to add a test to repro the mentioned error->fix in the description?

@SebastianMunozP SebastianMunozP changed the title [RSDK-12899] Fix get device properties [RSDK-12899] Fix set device properties Feb 3, 2026
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