-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove memcpy in HWM & command_transfer #12622
Conversation
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I'll remove
{ | ||
*p_response = err_type; | ||
return std::vector<uint8_t>(); | ||
return {}; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
@@ -204,7 +204,7 @@ namespace librealsense | |||
{} | |||
|
|||
std::vector<uint8_t> send_receive( | |||
const std::vector<uint8_t>& data, | |||
uint8_t const * pb, size_t cb, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What pb & cp means?
Don't you prefer meaningful names?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How PB & CP are data and data_size? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's pretty common Hungarian notation for "pointer to bytes" and "count of bytes".
size_t receivedCommandDataLength; | ||
uint8_t receivedOpcode[4]; | ||
|
||
explicit hwmon_cmd(uint8_t cmd_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we use it?
Python maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope. No longer in use (I removed the only usage).
Just some optimization when going over the code.
Also removed disruptive debug messages and expected throws for two options, and the
hwmon_cmd
struct.Instead,
rs2_create_error
now logs these errors.