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

Fix D457 RGB sensor exposure control range issue #12251

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

hsuys
Copy link
Contributor

@hsuys hsuys commented Oct 3, 2023

fix d457 rgb sensor exposure ctrl range issue

changes:
change to VIDIOC_QUERY_EXT_CTRL instead of VIDIOC_QUERYCTRL when calling ioctls

Tracked on: RSDSO-19295

query.id = get_cid(option);
if (xioctl(_fd, VIDIOC_QUERYCTRL, &query) < 0)
if (xioctl(_fd, VIDIOC_QUERY_EXT_CTRL, &query) < 0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will this affect other controls as well?

Does this means this comment will be no longer relevant?

// Some controls (exposure, auto exposure, auto hue) do not seem to work on V4L2
                // Instead of throwing an error, return an empty range. This will cause this control to be omitted on our UI sample.
                // TODO: Figure out what can be done about these options and make this work

Copy link
Collaborator

Choose a reason for hiding this comment

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

@hsuys ?

@dmipx can you review this change as well?
It should work with Ubuntu and JetPack w/o any regressions

@SamerKhshiboun
Copy link
Contributor

@dmipx can you please review this ?
ROS users already complained about undefined range for rgb exposure.

@Nir-Az
Copy link
Collaborator

Nir-Az commented Jun 4, 2024

Closed and reopened to trigger CI

@Nir-Az
Copy link
Collaborator

Nir-Az commented Jun 4, 2024

@hsuys Verified on JP4, Any explanation to why it works? :)
According to the control description it support a control array, I wasn't expecting a logic change.

@hsuys
Copy link
Contributor Author

hsuys commented Jun 4, 2024

I can confirm that this change still works for LRS 2.55.1 on JP5.0.2 and v1.0.1.21 driver with D457 mipi camera. Sorry, I couldn't really explain why this would work except that the same control was used for get_xu_range()

@Nir-Az
Copy link
Collaborator

Nir-Az commented Jun 5, 2024

@hsuys since we cannot explain why it works I recommend minimize the change and to only change the mipi get_pu_range function to use it, instead if the main uvc function.
This way we have minimal impact.
Thoughts?

@hsuys
Copy link
Contributor Author

hsuys commented Jun 6, 2024

@Nir-Az, I agree. This fix should be for mipi sku to minimize the impact.

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.

None yet

3 participants