Skip to content

debug_interface impl for DDS; rs-terminal with DDS - #12270

Merged
maloel merged 10 commits into
realsenseai:developmentfrom
maloel:hwm
Oct 11, 2023
Merged

debug_interface impl for DDS; rs-terminal with DDS#12270
maloel merged 10 commits into
realsenseai:developmentfrom
maloel:hwm

Conversation

@maloel

@maloel maloel commented Oct 9, 2023

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
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.

Comment thread src/hw-monitor.h
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
Copy Markdown
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
Copy Markdown
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.

@OhadMeir OhadMeir left a comment

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.

Looks great

@maloel
maloel merged commit 2c31402 into realsenseai: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