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

Test gazebo #70

Merged
merged 37 commits into from
Oct 11, 2023
Merged

Test gazebo #70

merged 37 commits into from
Oct 11, 2023

Conversation

delihus
Copy link
Contributor

@delihus delihus commented Sep 4, 2023

bump::patch

Signed-off-by: Jakub Delicat <[email protected]>
Signed-off-by: Jakub Delicat <[email protected]>
@rafal-gorecki rafal-gorecki marked this pull request as ready for review September 21, 2023 09:37
Comment on lines 41 to 47
def set_and_publish_velocities(
self, goal_x_distance, goal_y_distance, goal_theta_angle
):
self.goal_x_distance = goal_x_distance
self.goal_y_distance = goal_y_distance
self.goal_theta_angle = goal_theta_angle
self.publish_cmd_vel_messages()
Copy link
Contributor

@rafal-gorecki rafal-gorecki Sep 21, 2023

Choose a reason for hiding this comment

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

It would be useful to establish a name. The name of the function suggests set_velocities and the attributes are called distance.

Copy link
Contributor

@rafal-gorecki rafal-gorecki left a comment

Choose a reason for hiding this comment

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

I would like you to correct the names of the functions.

My proposal: I propose that this test only checks speed.
Let's say the speed should be within +- 5% of the set value. (I think that such a test will also be useful in the future when creating tests for a physical platform)

Additionally, in the future, separate position tests could be performed, but they would include nav2 and slam and would involve reaching the indicated point with a specified tolerance. However, it can be difficult to evaluate such a test.

@delihus
Copy link
Contributor Author

delihus commented Sep 21, 2023

@rafal-gorecki
I added velocities variables.

In regards to the test speed, the higher applications such as nav2 uses transforms published by the EKF (base_link -> odom) where useful information is a position. I would leave the position test.

Maybe, should we listen to tf instead of listening to topic?

Signed-off-by: Jakub Delicat <[email protected]>
@rafal-gorecki rafal-gorecki merged commit e3d48a2 into humble Oct 11, 2023
3 checks passed
@rafal-gorecki rafal-gorecki deleted the test-gazebo branch October 11, 2023 09:19
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