Skip to content

Vehicle to Start Frame Gen#125

Closed
nvikramraj wants to merge 3 commits intoPerceptionB_mainfrom
perceptionb_start_frame
Closed

Vehicle to Start Frame Gen#125
nvikramraj wants to merge 3 commits intoPerceptionB_mainfrom
perceptionb_start_frame

Conversation

@nvikramraj
Copy link
Collaborator

Create a Transformation matrix to convert vehicle frame points to start frame for pedestrian tracking

@nvikramraj nvikramraj requested review from KenC1014, LucasEby and lukasdumasius and removed request for krishauser February 20, 2025 06:39
Copy link
Collaborator

@LucasEby LucasEby left a comment

Choose a reason for hiding this comment

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

I think there are a few bugs in here outside of what we've talked about. I was trying to be thorough.
I did not double check your transforms.

)
speed = np.sqrt(gnss_msg.ve**2 + gnss_msg.vn**2)
callback(cv_image,points,GNSSReading(pose,speed,('error' if gnss_msg.error else 'ok')))
topic1 = self.ros_sensor_topics['front_camera']
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you label the topics with camera for example instead of numbering them? This is very minor just would be slightly easier to follow

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure

'''

# T World to Start
start_x, start_y, start_z = self.current_vehicle_state.pose.x, self.current_vehicle_state.pose.y, self.current_vehicle_state.pose.z
Copy link
Collaborator

Choose a reason for hiding this comment

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

This line here is confusing which is why I think it might be a bug:
start_x, start_y, start_z implies that the variables are in the start frame
self.current_vehicle_state implies that the object grabs the pose of the current frame of the vehicle which would not be start frame

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 function will not be used

self.current_vehicle_state.v = vehicle_state.speed
self.current_vehicle_state.to_frame(frame=ObjectFrameEnum.START, current_pose=self.current_vehicle_state.pose, start_pose_abs=self.start_pose_abs)
else:
raise Exception('Unable to get current pose of vehicle, lost GPS')
Copy link
Collaborator

Choose a reason for hiding this comment

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

I love it that you used an exception here. I think there are a lot of places where we should be using them to check for edge cases. This might be a confusing exception for the other teams though.

It may potentially be a better idea to log this, print it to the console, and throw out the currently scanned in pedestrians because we can't calculate their positions relative to start frame. This might be a good question to ask your roommate what he thinks?

@sonarqubecloud
Copy link

Copy link
Collaborator

@LucasEby LucasEby left a comment

Choose a reason for hiding this comment

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

Code looks fine from what I can tell.
I'll refactor the find_vels_and_ids function to accept start frame data


# To be checked with Lucas
# self.prev_agents = self.current_agents.copy()
self.current_agents.clear()
Copy link
Collaborator

Choose a reason for hiding this comment

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

This little bit here is good. I'll modify find_vels_and_ids to accept start frame data instead of current frame data

@nvikramraj nvikramraj closed this Feb 22, 2025
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