Skip to content

Conversation

tomerashuach
Copy link
Contributor

@tomerashuach tomerashuach commented Aug 25, 2021

added a FW check when the device is in recovery mode

Edit: added a heuristic FW size check to sr3xxx when updating in recovery mode.

Tested on: D435i, D405, L515, SR300.

Tracked on: DSO-17448

@tomerashuach tomerashuach reopened this Aug 29, 2021
@tomerashuach tomerashuach reopened this Aug 29, 2021
@tomerashuach tomerashuach marked this pull request as ready for review August 29, 2021 11:36
@tomerashuach tomerashuach marked this pull request as draft August 29, 2021 11:37
auto device_pid = _usb_device->get_info().pid;
auto it = ds::device_to_fw_min_version.find(_usb_device->get_info().pid);
if (it == ds::device_to_fw_min_version.end())
throw std::runtime_error(to_string() << "Min and Max firmware versions have not been defined for this device: " << device_pid);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Print the device ID as "0xABCD" hex

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

namespace librealsense
{
class updatable
class fw_checkable
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pls rename to firmware_image_check or similar

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed to "firmware_check_interface"

virtual void update_flash(const std::vector<uint8_t>& image, update_progress_callback_ptr callback, int update_mode) = 0;
virtual bool check_fw_compatibility(const std::vector<uint8_t>& image) const = 0;
std::string extract_firmware_version_string(const void* fw_image, size_t fw_image_size) const
static std::string extract_firmware_version_string(const void* fw_image, size_t fw_image_size)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you can pass it in as a vector<uint8_t> vector instead of C-types

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


// advanced SR3XX devices do not fit the "old" fw versions and
// legacy SR3XX devices do not fit the "new" fw versions
return (firmware_version(fw_version) >= firmware_version(min_max_fw_it->second.first)) &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

In case the condition doesn't hold - print to log the version number extracted from the FW image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

@tomerashuach tomerashuach force-pushed the firmware_update_recovery_17448 branch from fe7cbc3 to bf3febf Compare September 9, 2021 11:28
Copy link
Collaborator

@ev-mp ev-mp left a comment

Choose a reason for hiding this comment

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

Minor changes + max value for SR300 to be verified

bool result = (firmware_version(fw_version) >= firmware_version(min_max_fw_it->second.first)) &&
(firmware_version(fw_version) <= firmware_version(min_max_fw_it->second.second));
if (!result)
LOG_ERROR(fw_version);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pls elaborate the error details

bool result = (firmware_version(fw_version) >= firmware_version(min_max_fw_it->second.first)) &&
(firmware_version(fw_version) <= firmware_version(min_max_fw_it->second.second));
if (!result)
LOG_ERROR(fw_version);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here as well

{ SR306_PID, {"3.28.3.0", "99.99.99.99"}},
{ SR306_PID_DBG, {"3.28.3.0", "99.99.99.99"}},
{ SR300_RECOVERY, {"3.21.0.0", "99.99.99.99"}}
{ SR300v2_PID, {"3.27.0.0", "4.99.99.99"}},
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it should be 3.xx, not 4

#include "l500-fw-update-device.h"
#include "l500-private.h"


Copy link
Collaborator

Choose a reason for hiding this comment

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

Unintended change, pls drop

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.

9 participants