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

bugfix/get-config problem with TEXT type #8

Merged
merged 1 commit into from
Apr 30, 2019
Merged

bugfix/get-config problem with TEXT type #8

merged 1 commit into from
Apr 30, 2019

Conversation

babisboloudakis
Copy link
Contributor

@babisboloudakis babisboloudakis commented Apr 17, 2019

fixes #7

Fix a problem where, when trying to call the /get-config
service on a text field, you would first get wrong data,
and on the second call the node would crash.

Fix a problem where, when trying to call the /get-config
service on a text field, you would first get wrong data,
and on the second call the node would crash.
@babisboloudakis babisboloudakis changed the title Fix /get-config problem with TEXT type bugfix/get-config problem with TEXT type Apr 17, 2019
@Karsten1987
Copy link
Contributor

I hope you don't mind me editing your post. I've linked it to the issue you opened because I think they are related. Let me know if that's incorrect.

Also, can you somehow tell me how to test this PR?

@babisboloudakis
Copy link
Contributor Author

Indeed, this PR attempts to solve the issue, thanks for linking.

In order to test it, I suggest that you start a photo_node with a camera connected. Then try to call the /photo/get_config service on a TEXT configuration ( like artist, cameramodel, ownername etc. )
For example:
$ rosservice call /photo/get_config "param: 'artist'"
This should return something like:
value: "<camera's configuration>"

If you try the same on the previous version you should get 'junk' data on the first service call, and a segmentation fault on the second call.

@Karsten1987 Karsten1987 merged commit 9000f0c into bosch-ros-pkg:melodic-devel Apr 30, 2019
@Karsten1987
Copy link
Contributor

thanks for the patch!

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.

Problem using TEXT configurations
2 participants