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

Added a write parameter for the Mock Global Path node #223

Merged
merged 6 commits into from
Jan 9, 2024

Conversation

SPDonaghy
Copy link
Contributor

Description

Linked to Issue 49 in Local Pathfinding.

The purpose was to add a parameter to determine whether the generate_path function in node_mock_global_path.py should write a generated path to a new csv file.

The added parameter is:
write

Verification

  1. Run the global or local pathfinding launch file with development mode (default)
    ros2 launch local_pathfinding main_launch.py
    
  2. In a different terminal run some ros2 param commands
    ros2 param get /mgp_main global_path_filepath
    ros2 param set /mgp_main global_path_filepath /workspaces/sailbot_workspace/src/local_pathfinding/global_paths/path_2.csv
    

Resources

  • Link to the PR for this issue.

@SPDonaghy SPDonaghy self-assigned this Dec 10, 2023
@patrick-5546 patrick-5546 force-pushed the user/SPDonaghy/add_new_global_params branch from 8239982 to 12d2a21 Compare December 10, 2023 07:03
@patrick-5546
Copy link
Member

patrick-5546 commented Dec 10, 2023

I rebased this branch so that it only contains new commits that aren't already in other PR's that were squashed and merged into main. Also changed float to double to match to convention set out by the other parameters

@SPDonaghy
Copy link
Contributor Author

How much do you think the GPS position should change for us to want to generate a new global path? I am just wanting to set a default value for a gps_threshold param in the yaml file

@patrick-5546
Copy link
Member

How much do you think the GPS position should change for us to want to generate a new global path?

@jamenkaye any ideas? Some ideas that come to mind:

  • If the current position is greater than x * interval_spacing away from the closet global waypoint
  • Time based, every x minutes

And just thought of this, is there a way to force update the path even if it stays the same? I guess saving the file would do this?

@SPDonaghy
Copy link
Contributor Author

SPDonaghy commented Dec 10, 2023

And just thought of this, is there a way to force update the path even if it stays the same? I guess saving the file would do this?

Could we implement a service to run global_path_callback() on request, and request it from the command line with rosservice?

Saving the csv file would also do it though.

@patrick-5546
Copy link
Member

Could we implement a service to run global_path_callback() on request, and request it from the command line with rosservice?

Ooh interesting idea, I think that could work. As long as it's not too complicated, since saving the file already works. Also would want to run some version of the callback without all the checks right

@patrick-5546 patrick-5546 force-pushed the user/SPDonaghy/add_new_global_params branch from 63de7d6 to 5c15783 Compare December 10, 2023 18:47
@SPDonaghy
Copy link
Contributor Author

  • If the current position is greater than x * interval_spacing away from the closet global waypoint

Should we skip adding another parameter, and just use interval_spacing as the threshold distance?

@patrick-5546
Copy link
Member

Should we skip adding another parameter, and just use interval_spacing as the threshold distance?

I'm just worried if we use interval spacing directly then we could run into some trouble if right when we reach one global waypoint we go back a bit, which would cause the path to regenerate

@SPDonaghy
Copy link
Contributor Author

okay, sounds good, I will keep it in there

@patrick-5546
Copy link
Member

If the current position is greater than x * interval_spacing away from the closet global waypoint

This idea would make x the parameter and have it be some float > 1 so that it is a function of interval_spacing. What do you think about that?

@SPDonaghy
Copy link
Contributor Author

This idea would make x the parameter and have it be some float > 1 so that it is a function of interval_spacing. What do you think about that?

Sounds good! I made it 2 for now

@jamenkaye
Copy link
Contributor

Sorry I'm a little late to the party! I wanted to focus on exams.

I like the x parameter and inteval_spacing idea! I think something around 1-1.5 should be good. If $x \geq 1$, this lets us sail at no more than 60° upwind (which is pretty easy) and use only 1 tack to get to the next waypoint, even if the global path is heading straight upwind.

image

(Since the angles are all 60°, its an equilateral triangle so the spacing between waypoints is the same as the distance from the boat at the rightmost corner to the waypoint)

I'm a little unclear on the context for this feature though. Are we still planning to manually send the global path in deployment, or is this an autonomous global pathfinding that just plots a straight line? If its the latter, that would seem a little silly considering we already have a fancy global pathfinding that uses weather data too.

@SPDonaghy
Copy link
Contributor Author

SPDonaghy commented Dec 17, 2023

or is this an autonomous global pathfinding that just plots a straight line? If its the latter, that would seem a little silly considering we already have a fancy global pathfinding that uses weather data too.

Aside from the mgp node, what I've made so far is a custom path builder, and an interpolation function that fills in any intervals (which are more than interval_spacing apart) with a straight, evenly spaced sub path. While I think everything I've built so far is just inteded for mocks, I think there could be a use case for what I made in deployment, if we want to specify more complicated paths than just start->end, for example.

@jamenkaye
Copy link
Contributor

Ah makes sense, thanks for explaining! 👍

@SPDonaghy SPDonaghy force-pushed the user/SPDonaghy/add_new_global_params branch from 43e29bf to 7569e61 Compare January 9, 2024 18:21
@SPDonaghy SPDonaghy enabled auto-merge (squash) January 9, 2024 18:21
@SPDonaghy SPDonaghy merged commit e196764 into main Jan 9, 2024
12 checks passed
@SPDonaghy SPDonaghy deleted the user/SPDonaghy/add_new_global_params branch January 9, 2024 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants