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

Feature/swath discretization #93

Merged

Conversation

Marnonel6
Copy link
Contributor

Description:

This PR introduces a new feature in our Path Planning module which discretizes swath sections of the path. By providing a discretization step and the robot's swath speed, the function returns a new path with the swath sections discretized.

Main Changes:

Swath Discretization Function:

  • Added a new method Path::discretize_swath(double step_size, double swath_speed) that takes in a step_size and swath_speed and returns a new path with discretized swath sections.
  • The method works by iterating through the path points and discretizing swath points using the given step_size. The velocity of the swath is set based on the provided swath_speed.

Example Script:

  • An example script discretize_swath_path.cpp demonstrates the use of the discretize_swath function.
  • This script defines a field, a robot, and generates swath connections using Dubins curves. After which, it uses the new discretize_swath function to generate a discretized path and visualize it using our visualization module.

Tests:

  • Different swaft, headland and path planning parameters were tested.
  • The discretized path was visualized and inspected for correct discretization.
  • Graphs were also plotted of the coordinates and the angle to ensure correctness.

Screenshots & Files

image
discretized_swath_path.csv

Copy link
Member

@Gonzalo-Mier Gonzalo-Mier left a comment

Choose a reason for hiding this comment

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

Hi @Marnonel6, cool PR!! Thank you for your contribution. I have left a few comments to solve a few issues I've seen. Thank you again

src/fields2cover/types/Path.cpp Show resolved Hide resolved
src/fields2cover/types/Path.cpp Outdated Show resolved Hide resolved
src/fields2cover/types/Path.cpp Outdated Show resolved Hide resolved
src/fields2cover/types/Path.cpp Outdated Show resolved Hide resolved
src/fields2cover/types/Path.cpp Outdated Show resolved Hide resolved
src/fields2cover/types/Path.cpp Outdated Show resolved Hide resolved
src/fields2cover/types/Path.cpp Outdated Show resolved Hide resolved
src/fields2cover/types/Path.cpp Outdated Show resolved Hide resolved
src/fields2cover/types/Path.cpp Show resolved Hide resolved
tutorials/discretize_swath_path.cpp Outdated Show resolved Hide resolved
@Marnonel6
Copy link
Contributor Author

Marnonel6 commented Oct 2, 2023

I'll get on those comments thank you!

@Marnonel6
Copy link
Contributor Author

Updated some of the mentioned comments. Let me know what you think and what should still change @Gonzalo-Mier

Copy link
Member

@Gonzalo-Mier Gonzalo-Mier left a comment

Choose a reason for hiding this comment

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

Great! Could you add some test to to check that it works as expected in any future version? Thank you

src/fields2cover/types/Path.cpp Show resolved Hide resolved
src/fields2cover/types/Path.cpp Outdated Show resolved Hide resolved
@Marnonel6
Copy link
Contributor Author

Marnonel6 commented Oct 3, 2023

Okay I've updated your latest comments and added a bunch of tests. Let me know what you think @Gonzalo-Mier .

@Gonzalo-Mier Gonzalo-Mier merged commit b8c89ea into Fields2Cover:main Oct 3, 2023
131 checks passed
@Gonzalo-Mier
Copy link
Member

Thank your really much. Just for the record, I made a few changes. I changed the style of the code to be in the same style as the rest of the repo (2 spaces instead of 4 and { in the same line as for() and if()). Moreover, I moved the test from the path_planning to the Path_test.
Great job!!

@Marnonel6
Copy link
Contributor Author

My pleasure. Thank you for the merge!

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

2 participants