Skip to content
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

JSON-based device-info & fix for no 'dds' settings #12540

Merged
merged 12 commits into from
Jan 1, 2024

Conversation

maloel
Copy link
Collaborator

@maloel maloel commented Dec 26, 2023

With DFU, we need to communicate more fields, and maybe even custom fields.
The device-info is now just a JSON proxy.

There was a bug introduced recently with "dds": false in the settings. This includes a fix.

Other minor stuff - go by commit.

Tracked on [LRS-989]

@maloel maloel requested a review from OhadMeir December 26, 2023 11:23
//bool locked = true;

std::string const & name() const;
void set_name( std::string && );
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure from user perspective that rvalue is always desired, for instance rs2_device_to_info copies the name

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I debated but eventually I just decided that it's fine if we copied before calling the function. Otherwise I can change to const &. What would you prefer?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I prefer const &

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This means always a copy vs. a move if possible. But OK, done.


// FW update ID is a must for DFU; all cameras should have one
std::string const serial_number = dev.get_info( RS2_CAMERA_INFO_FIRMWARE_UPDATE_ID );
j["fw-update-id"] = serial_number;
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we want to check support for RS2_CAMERA_INFO_FIRMWARE_UPDATE_ID? Maybe there is a different ID

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

All our cameras have a FW update ID. I'll add the supports anyway.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The serial number is actually used to set up the topic root path. It's quite necessary. I think I'll leave as-is so it'll throw if it doesn't exist.

/**
* \return the one-line description: "[<type>] <name> s/n <#>"
*/
std::string get_description() const
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we need a C API corresponding to all CPP APIs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think so, though we could certainly do it -- this only sits at the top of the hourglass

Copy link
Contributor

Choose a reason for hiding this comment

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

According to Evgeni we need to be able to do in C whatever we do in CPP.
The APIs should be synchronized and the CPP just a wrapper to the C

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But it is: this is using the C APIs. I am not adding another API here. I am making the code reusable. Are you suggesting I add yet another API that does this inside librealsense?

@maloel maloel merged commit 4d12d4f into IntelRealSense:development Jan 1, 2024
16 of 17 checks passed
@maloel maloel deleted the device-info branch January 1, 2024 06:02
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