Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
hwm remove 3 copies and many log msgs
  • Loading branch information
maloel committed Jan 31, 2024
commit e6cd0e282308e7c39ac279eaf0ab2e632de6b36b
2 changes: 1 addition & 1 deletion src/ds/d500/hw_monitor_extended_buffers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ namespace librealsense
// - no buffer bigger than 1 KB is expected with the current command => work as normal hw_monitor::send method
// - buffer bigger than 1 KB expected to be received => iterate the hw_monitor::send method and append the results
// - buffer bigger than 1 KB expected to be sent => iterate the hw_monitor, while iterating over the input
std::vector<uint8_t> hw_monitor_extended_buffers::send(command cmd, hwmon_response* p_response, bool locked_transfer) const
std::vector<uint8_t> hw_monitor_extended_buffers::send(command const & cmd, hwmon_response* p_response, bool locked_transfer) const
{
hwm_buffer_type buffer_type = get_buffer_type(cmd);
if (buffer_type == hwm_buffer_type::standard)
Expand Down
2 changes: 1 addition & 1 deletion src/ds/d500/hw_monitor_extended_buffers.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ namespace librealsense
{}

virtual std::vector<uint8_t> send(std::vector<uint8_t> const& data) const override;
virtual std::vector<uint8_t> send(command cmd, hwmon_response* = nullptr, bool locked_transfer = false) const override;
virtual std::vector<uint8_t> send(command const & cmd, hwmon_response* = nullptr, bool locked_transfer = false) const override;

private:
int get_msg_length(command cmd) const;
Expand Down
24 changes: 17 additions & 7 deletions src/ds/ds-options.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -550,16 +550,26 @@ namespace librealsense
{
float rv = 0.f;
command cmd(ds::GETSUBPRESETID);
// if no subpreset is streaming, the firmware returns "ON_DATA_TO_RETURN" error
try {
auto res = _hwm.send(cmd);
// if a subpreset is streaming, checking this is the alternating emitter sub preset
if (res.size())
rv = (res[0] == ds::ALTERNATING_EMITTER_SUBPRESET_ID) ? 1.0f : 0.f;
try
{
hwmon_response response;
auto res = _hwm.send( cmd, &response ); // avoid the throw
Comment thread
OhadMeir marked this conversation as resolved.
switch( response )
{
case hwmon_response::hwm_NoDataToReturn:
// If no subpreset is streaming, the firmware returns "NO_DATA_TO_RETURN" error
break;
default:
// if a subpreset is streaming, checking this is the alternating emitter sub preset
if( res.size() )
rv = ( res[0] == ds::ALTERNATING_EMITTER_SUBPRESET_ID ) ? 1.0f : 0.f;
else
LOG_DEBUG( "alternating emitter query: " << hwmon_error_string( cmd, response ) );
break;
}
}
catch (...)
{
rv = 0.f;
}

return rv;
Expand Down
24 changes: 17 additions & 7 deletions src/hdr-config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -228,16 +228,26 @@ namespace librealsense
{
float rv = 0.f;
command cmd(ds::GETSUBPRESETID);
// if no subpreset is streaming, the firmware returns "ON_DATA_TO_RETURN" error
try {
auto res = _hwm.send(cmd);
// if a subpreset is streaming, checking this is the current HDR sub preset
if (res.size())
rv = (is_hdr_id(res[0])) ? 1.0f : 0.f;
try
{
hwmon_response response;
auto res = _hwm.send( cmd, &response ); // avoid the throw
Comment thread
OhadMeir marked this conversation as resolved.
switch( response )
{
case hwmon_response::hwm_NoDataToReturn:
// If no subpreset is streaming, the firmware returns "NO_DATA_TO_RETURN" error
break;
default:
// If a subpreset is streaming, checking this is the current HDR sub preset
if( res.size() )
rv = ( is_hdr_id( res[0] ) ) ? 1.0f : 0.f;
else
LOG_DEBUG( "hdr_config query: " << hwmon_error_string( cmd, response ) );
break;
}
}
catch (...)
{
rv = 0.f;
}

_is_enabled = (rv == 1.f);
Expand Down
51 changes: 22 additions & 29 deletions src/hw-monitor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,24 +143,23 @@ namespace librealsense
}

std::vector< uint8_t >
hw_monitor::send( command cmd, hwmon_response * p_response, bool locked_transfer ) const
hw_monitor::send( command const & cmd, hwmon_response * p_response, bool locked_transfer ) const
{
hwmon_cmd newCommand(cmd);
auto opCodeXmit = static_cast<uint32_t>(newCommand.cmd);
uint32_t const opCodeXmit = cmd.cmd;

hwmon_cmd_details details;
details.require_response = newCommand.require_response;
details.timeOut = newCommand.timeOut;

fill_usb_buffer(opCodeXmit,
newCommand.param1,
newCommand.param2,
newCommand.param3,
newCommand.param4,
newCommand.data,
newCommand.sizeOfSendCommandData,
details.sendCommandData.data(),
details.sizeOfSendCommandData);
details.require_response = cmd.require_response;
details.timeOut = cmd.timeout_ms;

fill_usb_buffer( opCodeXmit,
cmd.param1,
cmd.param2,
cmd.param3,
cmd.param4,
cmd.data.data(), // memcpy
std::min( (uint16_t)cmd.data.size(), HW_MONITOR_BUFFER_SIZE ),
details.sendCommandData.data(),
details.sizeOfSendCommandData );

if (locked_transfer)
{
Expand All @@ -172,33 +171,27 @@ namespace librealsense
// Error/exit conditions
if (p_response)
*p_response = hwm_Success;
if( !newCommand.require_response )
return std::vector<uint8_t>();

std::memcpy( newCommand.receivedOpcode, details.receivedOpcode.data(), 4 );
std::memcpy( newCommand.receivedCommandData,
details.receivedCommandData.data(),
details.receivedCommandDataLength );
newCommand.receivedCommandDataLength = details.receivedCommandDataLength;
if( ! cmd.require_response )
return {};

// endian?
auto opCodeAsUint32 = pack(details.receivedOpcode[3], details.receivedOpcode[2],
details.receivedOpcode[1], details.receivedOpcode[0]);
if (opCodeAsUint32 != opCodeXmit)
{
auto err_type = static_cast<hwmon_response>(opCodeAsUint32);
std::string err = hwmon_error_string(cmd, err_type);
LOG_DEBUG(err);
if (p_response)
//LOG_DEBUG(err); // too intrusive; may be an expected error
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.

You removed the line that initialize err

std::string err = hwmon_error_string(cmd, err_type);

Better to keep as comment or remove both, right?

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.

OK, I'll remove

if( p_response )
{
*p_response = err_type;
return std::vector<uint8_t>();
return {};
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.

Just to understand, why is this better than before? what happens behind the scene?

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.

They're exactly the same, except the latter will still work if we change the return type of the function.

}
std::string err = hwmon_error_string( cmd, err_type );
throw invalid_value_exception(err);
}

return std::vector<uint8_t>(newCommand.receivedCommandData,
newCommand.receivedCommandData + newCommand.receivedCommandDataLength);
auto const pb = details.receivedCommandData.data();
return std::vector<uint8_t>( pb, pb + details.receivedCommandDataLength );
}

/*static*/ std::vector<uint8_t> hw_monitor::build_command(uint32_t opcode,
Expand Down
2 changes: 1 addition & 1 deletion src/hw-monitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ namespace librealsense
static command build_command_from_data(const std::vector<uint8_t> data);

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;
virtual std::vector<uint8_t> send( command const & cmd, hwmon_response * = nullptr, bool locked_transfer = false ) const;
static std::vector<uint8_t> build_command(uint32_t opcode,
uint32_t param1 = 0,
uint32_t param2 = 0,
Expand Down
5 changes: 4 additions & 1 deletion src/types.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,10 @@ namespace librealsense
rs2_exception_type exception_type) noexcept
: librealsense_exception(msg, exception_type)
{
LOG_DEBUG("recoverable_exception: " << msg);
// This is too intrusive: some throws are completely valid and even expected (options-watcher queries all
// options, some of which may not be queryable) and this just dirties up the logs. It doesn't have the actual
// exception type, either.
//LOG_DEBUG("recoverable_exception: " << msg);
}

}