Skip to content

Conversation

@maloel
Copy link
Contributor

@maloel maloel commented Nov 7, 2023

Still some stuff left, but I figured this was enough for now (it's been building up).
Some are bigger than others. I recommend comparing commits one at a time to make this more understandable.

No functional changes.

Tracked on [LRS-923]

@maloel maloel requested a review from OhadMeir November 7, 2023 12:50
#include <numeric>
#include "reflectivity.h"
#include <types.h>
#include <vector>
Copy link
Collaborator

Choose a reason for hiding this comment

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

BTW, Reflectivity & MUR can both be deleted totally., not used.
Both are L515 only and was origin by L515 reflectivity issues

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I know, we spoke about it and, at least for now, I decided to leave them alone. Let's discuss offline.

<< intrinsics_string(res_1280_800)
<< intrinsics_string(res_960_540));

#undef intrinsics_string
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this needed? even on cpp file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, just a good practice

namespace librealsense {


using firmware_version = rsutils::version;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be a problem.
Better to specifically use rsutils::version,
firmware_version is spread around the code as local var and we always do using librealsense

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just moved it in this PR. If you want I can work to change but it's a whole lot of changes in many many places.


frame_callback_ptr rv = {
new internal_frame_callback<decltype(to_syncer)>(to_syncer),
[](rs2_frame_callback* p) { p->release(); }
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need for this release anymore?
Just need to make sure no memory leak here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's still there: make_frame_callback adds it automatically.

@maloel maloel merged commit 5026b67 into IntelRealSense:development Nov 7, 2023
@maloel maloel deleted the types-h-mid branch November 7, 2023 18:40
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