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

Bug fix for fields in service calls with different names. #347

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

tompe17
Copy link

@tompe17 tompe17 commented Mar 21, 2022

This is my fix for the issue in #285

@fujitatomoya
Copy link
Collaborator

@tompe17 tompe17 force-pushed the fieldnamebug branch 2 times, most recently from b9c8200 to f456625 Compare March 22, 2022 08:08
ros1_bridge/__init__.py Outdated Show resolved Hide resolved
@tompe17 tompe17 force-pushed the fieldnamebug branch 2 times, most recently from c49c0e5 to b6abc94 Compare March 25, 2022 11:55
gbiggs and others added 4 commits March 25, 2022 12:56
* Update package maintainers

Signed-off-by: Geoffrey Biggs <[email protected]>
Signed-off-by: Tommy Persson <[email protected]>
* Fix cpplint error

Signed-off-by: Jorge Perez <[email protected]>
Signed-off-by: Tommy Persson <[email protected]>
* Parametrizing service execution timeout

Signed-off-by: Marco Bassa <[email protected]>
Signed-off-by: Tommy Persson <[email protected]>
Signed-off-by: Tommy Persson <@[email protected]>
Signed-off-by: Tommy Persson <[email protected]>
@gbiggs
Copy link
Member

gbiggs commented Mar 25, 2022

CI:

  • Linux Build Status

Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

just curious, why are there unrelated changes which are already in master branch?

@tompe17
Copy link
Author

tompe17 commented Apr 10, 2022

Do I need to do anything more with this or will this be merged? It would very good if this functionality could work in the next release of ROS2 since we will depend on it until DJI releases it ROS SDK for ROS2 if that ever will happen.

@gbiggs
Copy link
Member

gbiggs commented Apr 10, 2022

@tompe17 You have been asked a question by @fujitatomoya. I'm waiting for your reply before merging this.

@tompe17
Copy link
Author

tompe17 commented Apr 19, 2022

Concerning unrelated changed since I have not the faintest idea about why that is I could not answer it. I can only describe what I did:

  • I forked the repo
  • I made a branch and did the changes
  • I made the pull request
  • There was the issue with a signature so I followed the instructions to fix that. Some day after that I for a messaged that the email address was invalid since it by mistake started with @.
  • So i tried to fix that and repeted the original fix that was suggested after fixing my buggy email address I had in a config file.
  • I tried that a couple of times since it did not seem to help. The I saw that @gbiggs hade approves something so I thought everything was OK.

I am not familiar enough with github and pull requests to understand if what I did have caused some problems or not.

@gbiggs
Copy link
Member

gbiggs commented May 5, 2022

@fujitatomoya Could you be specific about which changes you think are unrelated?

@methylDragon
Copy link
Contributor

@gbiggs

My suspicion is these:
image

Suggestion: Rebase the branch on master, which should remove a lot of the extra commits, it'll also fix the conflicts since the creation of this PR.

@gbiggs
Copy link
Member

gbiggs commented Jun 29, 2022

@tompe17 Can you please rebase your PR?

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.

6 participants