Skip to content

Introducing CuTimeRange#106

Merged
gbin merged 4 commits into
masterfrom
gbin/tov-range
Nov 21, 2024
Merged

Introducing CuTimeRange#106
gbin merged 4 commits into
masterfrom
gbin/tov-range

Conversation

@gbin

@gbin gbin commented Nov 19, 2024

Copy link
Copy Markdown
Collaborator

This is needed to represent messages that contains more than 1 Tov like several images, several IMU measures etc... We will add helpers like range merging, range intersections etc in a separate PRs.

@gbin gbin requested a review from mik90 November 19, 2024 16:11
Comment thread components/sources/cu_hesai/src/lib.rs Outdated
new_msg.metadata.tov = payload.tov[0].into(); // take the oldest timestamp

let tov_range = CuTimeRange {
start: payload.tov[0],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Since this is a soa, can we build a CuTimeRange from a slice?

So impl From or TryFrom for CuTimeRange and get CuTimeRange::from(&[T])

Syntactic sugar but still useful

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Added it. It is not used yet but it is a nice thing to have.

Comment thread components/tasks/cu_pid/src/lib.rs Outdated
let tov = input.metadata.tov.unwrap();
let tov = match input.metadata.tov {
Tov::Time(single) => single,
_ => panic!("Unexpected variant"),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is this panicking because nothing should ever continue if this happens?

An early-return with Err would be more configurable for error handling

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

indeed, fixed thanks

/// Represents a time range.
#[derive(Clone, Debug, Encode, Decode, Serialize, Deserialize)]
pub struct CuTimeRange {
pub start: CuTime,

@mik90 mik90 Nov 19, 2024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

CuTime is Copy, CuTimeRange can be copy too can't it https://doc.rust-lang.org/std/marker/trait.Copy.html#when-can-my-type-be-copy?

Ditto for the partial time range

This would remove the need to do clone() in the task code

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

good point, fixed.

gbin added 3 commits November 21, 2024 08:31
This is needed to represent messages that contains more than 1 Tov like several images, several IMU measures etc...
We will add helpers like range merging, range intersections etc in a separate PRs.
@gbin gbin merged commit f773fb0 into master Nov 21, 2024
@gbin gbin deleted the gbin/tov-range branch November 21, 2024 15:06
@makeecat makeecat added the enhancement New feature or request label Jan 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants