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

Proposing some minor changes #2

Merged
merged 15 commits into from
Oct 3, 2022
Merged

Conversation

ijnek
Copy link

@ijnek ijnek commented Aug 31, 2022

@Flova

The main change in this PR, is add default parameters for time and plane_frame_id.

There are also some simplifications that I made with the srv files, and cleaning up of package.xml files

@ijnek ijnek marked this pull request as ready for review August 31, 2022 23:36
Signed-off-by: Kenji Brameld <[email protected]>
@ijnek ijnek mentioned this pull request Aug 31, 2022
ipm_interfaces/package.xml Show resolved Hide resolved
ipm_library/ipm_library/ipm.py Show resolved Hide resolved
ipm_library/ipm_library/ipm.py Outdated Show resolved Hide resolved
ipm_library/package.xml Show resolved Hide resolved
ipm_library/package.xml Outdated Show resolved Hide resolved
ipm_library/test/test_ipm.py Outdated Show resolved Hide resolved
@ijnek
Copy link
Author

ijnek commented Sep 7, 2022

@Flova are there some actionables for me here?

@Flova
Copy link
Owner

Flova commented Sep 17, 2022

Not really, mainly only the API discussion.

Signed-off-by: Kenji Brameld <[email protected]>
Signed-off-by: Kenji Brameld <[email protected]>
@ijnek
Copy link
Author

ijnek commented Sep 20, 2022

@Flova I think I've addressed everything now. I've also reverted some formatting so the differences are easier to see.

ipm_library/test/test_ipm.py Outdated Show resolved Hide resolved
@Flova
Copy link
Owner

Flova commented Sep 30, 2022

@ijnek I think everything except for the test case is fine now.

Signed-off-by: Kenji Brameld <[email protected]>
@ijnek
Copy link
Author

ijnek commented Oct 3, 2022

@Flova Should be good now!

@Flova Flova merged commit 908dbfa into Flova:adapt_api Oct 3, 2022
@ijnek ijnek deleted the ijnek-adapt-api branch October 3, 2022 21:39
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.

3 participants