-
Notifications
You must be signed in to change notification settings - Fork 33
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
handle dynamic extents #15
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
christianrauch
force-pushed
the
fix_dynamic_extent
branch
3 times, most recently
from
October 30, 2023 22:41
ea3f6a5
to
56c9e13
Compare
christianrauch
changed the title
handle dynamic extents in cv_to_pv
handle dynamic extents
Oct 30, 2023
christianrauch
force-pushed
the
fix_dynamic_extent
branch
2 times, most recently
from
October 31, 2023 23:06
56c9e13
to
5e04f2d
Compare
christianrauch
force-pushed
the
fix_dynamic_extent
branch
from
May 5, 2024 16:17
680ebc2
to
055c1db
Compare
christianrauch
force-pushed
the
fix_dynamic_extent
branch
from
May 25, 2024 16:57
055c1db
to
9dc2b2c
Compare
christianrauch
force-pushed
the
fix_dynamic_extent
branch
5 times, most recently
from
June 12, 2024 18:28
121fd17
to
c4db1f2
Compare
christianrauch
force-pushed
the
fix_dynamic_extent
branch
4 times, most recently
from
June 22, 2024 11:33
c8ed14b
to
4873473
Compare
christianrauch
force-pushed
the
fix_dynamic_extent
branch
3 times, most recently
from
August 5, 2024 19:27
36b8504
to
bf1fa07
Compare
christianrauch
force-pushed
the
fix_dynamic_extent
branch
2 times, most recently
from
August 17, 2024 17:57
e040af9
to
31d860d
Compare
Dynamic Spans have a maximum extent ('dynamic_extent') but their associated 'ControlInfo' can contain a variable number of elements. For dynamic Spans with a single element in the default 'ControlValue', this previously caused the exception 'std::length_error' since a vector with "maximum extent" elements was constructed. Fix this by ignoring the extent and using the actual number of elements.
christianrauch
force-pushed
the
fix_dynamic_extent
branch
from
August 17, 2024 18:00
31d860d
to
6ca0d5c
Compare
Previously 'get_extent' would return the original 'extent' of a Span and 0 for non-span controls. This is ambiguous as an extent of 0 means that the original control type is either not a span or an empty span that can store no elements. Resolve this ambiguity by enforcing that libcamera controls cannot contain empty spans via a compile time assertion and clarify that an extent of 0 is only returned for non-span types.
christianrauch
force-pushed
the
fix_dynamic_extent
branch
from
August 17, 2024 18:06
6ca0d5c
to
9ff80dc
Compare
…value libcamera 'Control<T>' and their related 'ControlInfo' do not necessarily have the same control and value types. This previously caused issues when an unsupported control type, such as a span of a complex type, is mapped to a ROS parameter via the type of the default 'ControlValue'. Fix this by using the the actual control type, regardless of the types of the values in the 'ControlInfo'.
christianrauch
force-pushed
the
fix_dynamic_extent
branch
from
August 17, 2024 18:14
9ff80dc
to
0a74728
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Dynamic Spans have a maximum extent for the
Control<Span<Type>>
but a variable number of elements for theControlValue
in theControlInfo
. For dynamic Spans with a single element, this previously caused the exceptionstd::length_error
since a vector with "maximum extent" (dynamic_extent
) elements was constructed. Additionally, libcamera allows controls to have default and min/max values of different types than the control.This PR thus changes the behaviour of how libcamera controls are mapped and converted to ROS parameters in the following way:
ControlValue
and use the actual number of elements to prevent memory allocation with the "maximum extent"Control
contains a scalar type, such asControl<Type>
, and not a Span, such asControl<Span<Type>>
Control
type and not theControlValue
Altogether, this will ensure that we do not attempt to convert controls that are typed with an unsupported type, such as arrays of complex types (e.g.
Control<Span<const Rectangle>>
), independently of the variable types in theControlInfo
which is set by the IPA or pipeline.Fixes #13, Fixes #50.