Skip to content
This repository has been archived by the owner on Mar 23, 2024. It is now read-only.

Global Path HTTP Module #70

Merged

Conversation

SPDonaghy
Copy link
Contributor

@SPDonaghy SPDonaghy commented Jan 16, 2024

Description

Aside from the Issue, I made some small changes, here and there, to my work on the previous PR, just some renaming and set write to False in the mgp test file.

Verification

  • Pytests for new functions
  • Created mock HTTP server for endpoint for NET to test out the POST request made by the module, to send the global path to NET.
  • HTTP request test coverage also with Pytests

Resources

@SPDonaghy SPDonaghy self-assigned this Jan 16, 2024
@SPDonaghy SPDonaghy added the enhancement New feature or request label Jan 16, 2024
Copy link
Member

@patrick-5546 patrick-5546 left a comment

Choose a reason for hiding this comment

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

Great work! What do you think about importing common functions from global_path.py in node_mock_global_path.py instead of vice versa (how it currently is) so that it can be standalone. I think this would also require converting the ROS parameters you added for mgp_main to constants.

Also how about adding some functionality in node_mock_global_path.py to publish or post depending on a ros parameter

Let's discuss about integration with WEB and NET in the soft-wide meeting on Saturday (I added a discussion topic in the meeting notes, feel free to add to it)

local_pathfinding/node_mock_global_path.py Outdated Show resolved Hide resolved
@SPDonaghy
Copy link
Contributor Author

Great work! What do you think about importing common functions from global_path.py in node_mock_global_path.py instead of vice versa (how it currently is) so that it can be standalone. I think this would also require converting the ROS parameters you added for mgp_main to constants.

Also how about adding some functionality in node_mock_global_path.py to publish or post depending on a ros parameter

Let's discuss about integration with WEB and NET in the soft-wide meeting on Saturday (I added a discussion topic in the meeting notes, feel free to add to it)

Yeah it definitely makes more sense to do it the other way around, I agree. For sure, lets discuss it more in the meeting on Sat.

@patrick-5546
Copy link
Member

Following up on our discussion today, plan for tests is to read from DB directly instead of get_server.py, and to use post_server.py until NET has implemented their endpoint (let's create an issue to remove post_server.py once it is) - I mentioned this PR in a sailbot workspace PR that is required for CI to pass after making these changes

@patrick-5546
Copy link
Member

I'm guessing tests are still failing because ports aren't being forwarded. In sailbot workspace this is done in devcontainer.json, but that isn't used for running CI (just the compose files are)

@SPDonaghy
Copy link
Contributor Author

I'm guessing tests are still failing because ports aren't being forwarded. In sailbot workspace this is done in devcontainer.json, but that isn't used for running CI (just the compose files are)

Sorry I'm a little confused, what action is needed from me?

@patrick-5546
Copy link
Member

It was a note for myself, will fix it when I have time (sometime this week)

Copy link
Member

@patrick-5546 patrick-5546 left a comment

Choose a reason for hiding this comment

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

Finally got CI to pass. It would have been much simpler with a monorepo. Have you created an issue to use the NET endpoint once it's implemented yet?

@SPDonaghy SPDonaghy merged commit 61e7d47 into main Mar 8, 2024
12 checks passed
@SPDonaghy SPDonaghy deleted the user/SPDonaghy/Global-path-part-2-return-of-the-path-#69 branch March 8, 2024 23:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mock Global Path Part 2: WEB & NET
3 participants