ros-industrial / robotiq

Robotiq packages (http://wiki.ros.org/robotiq)
BSD 2-Clause "Simplified" License
229 stars 378 forks source link

Simultaneously using two FT-150s #113

Closed rkeatin3 closed 6 years ago

rkeatin3 commented 6 years ago

I'm trying to use two FT-150s from one computer, but there are problems with the code that currently prevent me from doing so:

  1. There isn't a way to specify which device the node should connect to. It looks like code linked to within Issue 107 solves that problem

  2. All of the functions and variables seem to be static... Has this been addressed before?

shaun-edwards commented 6 years ago

There isn't a way to specify which device the node should connect to. It looks like code linked to within Issue 107 solves that problem

If you could verify the fix, I'm happy to merge it. I don't have access to hardware, let alone multiple, so it's impossible for me to test

All of the functions and variables seem to be static... Has this been addressed before?

No...is it causing a problem?

rkeatin3 commented 6 years ago

I have verified that the code linked to in issue 107 does properly initialize the node with the device handle loaded into the param server. Perhaps the point made here is a good one though: this change is breaking in that it will force users to specify the device.

It looks like I have to eat crow on my second point, however: I expected the use of global variables would cause problems when running two nodes, but the data looks good from both of the devices I'm using.

shaun-edwards commented 6 years ago

@rkeatin3, can you think of a work around for the single device? Is there a way to make it backwards compatible?

rkeatin3 commented 6 years ago

Though I haven't tried, I imagine falling back to the default behavior if the rosparam hasn't been populated should work. The author of the fix certainly found that behavior contemptible, but I'm not sure what other options there are if the user isn't expected to know the path to the device file.

I can implement and test falling back to that behavior tomorrow if that's the desired fix.

rkeatin3 commented 6 years ago

It worked as expected. Should I create a pull request?

rkeatin3 commented 6 years ago

This issue has been resolved with Pull Request 114.