Skip to content

Conversation

@maloel
Copy link
Contributor

@maloel maloel commented Oct 9, 2023

Implement hwm control:

  • support in dds-adapter [LRS-897], dds_device_proxy [LRS-898]
  • rs-terminal support for DDS [LRS-896]
  • updated docs

On the way, minor stuff:

  • add motion in d555e create_matcher()
  • static hw_monitor::build_command()
  • fix topic-send script

@maloel maloel requested review from OhadMeir and qtsleng October 9, 2023 08:15
_dds_dev->send_control( control, &reply );
std::string default_status( "OK", 2 );
if( rsutils::json::get( reply, "status", default_status ) != default_status )
throw std::runtime_error( "Failed HWM: "
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't there an `explanation`` field in case of failure?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's what it's supposed to be getting in the next line... I copy-pasted, maybe there's a bug somewhere else, too -- I'll check.

virtual std::vector<uint8_t> send( std::vector<uint8_t> const & data ) const;
virtual std::vector<uint8_t> send( command cmd, hwmon_response * = nullptr, bool locked_transfer = false ) const;
std::vector<uint8_t> build_command(uint32_t opcode,
static std::vector<uint8_t> build_command(uint32_t opcode,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this as static?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the beginning, I had the build-command code directly call the hw-monitor, but there is no hw-monitor object in the DDS device. So how can you build a command?

Later I decided I didn't want the DDS code to even know what a hardware monitor is, and that we shouldn't assume to know how a device builds its commands, so I removed the hw-monitor references but this stayed.

Currently, I think it's fine making it static as internally that's how it implemented (it calls a static function). Otherwise we would have to turn it into a virtual function and do some other work. I think that it's valid to go that way, but in a different PR and only if needed.

Copy link
Contributor

@OhadMeir OhadMeir left a comment

Choose a reason for hiding this comment

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

Looks great

@maloel maloel merged commit 2c31402 into IntelRealSense:development Oct 11, 2023
@maloel maloel deleted the hwm branch October 11, 2023 08:34
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.

2 participants