[RSDK-13561] - Dedup device property setting#70
Conversation
| } else if (property_item.type == OB_BOOL_PROPERTY) { | ||
| if (!property_map.begin()->second.is_a<bool>()) { | ||
| return {{"error", "Invalid type for bool property"}}; | ||
| if (property_item.permission == OB_PERMISSION_DENY || property_item.permission == OB_PERMISSION_READ) { |
There was a problem hiding this comment.
Added check here to make sure we have write permissions
(Moved away from setDeviceProperties)
| device->setBoolProperty(property_item.id, bool_value); | ||
| } else { | ||
| return {{"error", "Unsupported property type"}}; | ||
| } catch (ob::Error& e) { |
There was a problem hiding this comment.
Added try/catch to cleanly return errors instead of unhandled exceptions.
| return {{"error", "properties must be a struct"}}; | ||
| } | ||
| std::unordered_set<std::string> writable_properties; | ||
| std::unordered_set<std::string> seen_properties; |
There was a problem hiding this comment.
Removed seen_properties, setDeviceProperty handles unsupported prop names
| return {{"error", "Invalid type for float property"}}; | ||
| } | ||
| float float_value = property_map.begin()->second.get_unchecked<double>(); | ||
| device->setFloatProperty(property_item.id, float_value); |
There was a problem hiding this comment.
[opt] I wonder if we could make a templated helper function to reduce the duplicate code between the different types
There was a problem hiding this comment.
This is a good idea, it would declutter this code a lot.
Sean, do you think you could do this as a follow up?, or in this PR?
There was a problem hiding this comment.
Might be better as a follow-up or maybe punt on this all together. Looks like the realsense SDK uses different setters per type (setFloatProperty, setIntProperty, setBoolProperty), so, it would NOT be a clean templated:
device->setProperty<int>(id, value);
device->setProperty<float>(id, value);
device->setProperty<bool>(id, value);
There was a problem hiding this comment.
Ok, added templated helper validateAndSetProperty that checks types and fans out to the corresponding realsense method.
|
Manually tested DoCommand with template header -- merging |
3 participants
Description
Simple PR to clean up device property setting
Testing
{"set_device_properties": {"OB_PROP_COLOR_AUTO_EXPOSURE_BOOL": {"current": false}, "OB_PROP_COLOR_EXPOSURE_INT": {"current": 33000}}}Screen.Recording.2026-04-15.at.3.00.11.PM.mov
Review
1 approval