search

ros-drivers / flir_camera_driver

163 stars 154 forks source link

fix parse serial hex from string #17

Closed furushchev closed 5 years ago

furushchev commented 5 years ago

Currently, the program parses serial string from ROS parameter and only cast into integer. This PR fixes to read serial string as hex integer.

furushchev commented 5 years ago

@mhosmar-cpr Kindly ping to maintainer.

mikehosmar commented 5 years ago

Hello @furushchev, Unfortunately this breaks down if the user intends to write the serial as a decimal integer. Considering the test spinnaker node and the example in stereo.launch both produce/use integers, adding this change would confuse more than it would help. Consider making a different ros parameter for hex serials. There is a method for reading hex serials from a file. This may work for your usecase. https://github.com/ros-drivers/flir_camera_driver/blob/94b5aa0e90fbe6089fd938e0efd439bde83b5078/spinnaker_camera_driver/src/nodelet.cpp#L285

furushchev commented 5 years ago

@mikehosmar Thank you for reviewing! I still have a question, I think that If the serial parameter value is given as a decimal integer, it should be read from the line: https://github.com/ros-drivers/flir_camera_driver/blob/kinetic-devel/spinnaker_camera_driver/src/nodelet.cpp#L264-L267 . If the value is given as a hex integer, then the parameter type is handled as string by the method. Is this correct? If not, what is the if condition of reading the parameter as a string for?

furushchev commented 5 years ago

@mikehosmar I found the serial number which is supposed to be used commonly is the number printed on the image sensor which is decimal. I used the serial number which can be found in dmesg displayed when the device is connected which is hex string. Now I'm ok to leave the code be without this PR. thank you for reviewing, anyway! :+1:

mikehosmar commented 5 years ago

@furushchev Thanks for sorting this out on your own. Your logic seems sound and I can understand the confusion. I agree that it makes sense to keep the serial section the same.