PickNikRobotics / ros2_robotiq_gripper

BSD 3-Clause "New" or "Revised" License
56 stars 43 forks source link

Potential use with Robotiq 3f? #39

Open umicharmlab opened 11 months ago

umicharmlab commented 11 months ago

Hello! I'm wondering if I can use this to control our 3f grippers. I believe this is for the 2f, so I would expect to need to do some work to make it work for 3f but so far this is the best/closest thing I've found to a ROS2 driver for the robotiq 3f.

I have the datasheet so I could presumably make a new driver class for 3f? I'd also need a urdf, which I can probably copy from the ros1 repo. Any major hurdles/roadblocks that I'm missing?

Are contributions are welcome? If I get this working should I make a PR to add this? or should it be a separate repo?

PeterMitrano commented 11 months ago

^ This was me, I was using our shared lab account :)

moriarty commented 11 months ago

I'm not sure, but we'd welcome PRs if you do get it working.

The idea behind this big PR: https://github.com/PickNikRobotics/ros2_robotiq_gripper/pull/34

Was to get this repo more in sync with another repo we've been working on and then having all the robotiq grippers in one place, and we would welcome the 3f gripper too we just don't have one physically.

moriarty commented 11 months ago

CC @kineticsystem thoughts/opinions?

PeterMitrano commented 10 months ago

I'm planning to start by just in-place updating things from 2f to 3f, but I would like to contribute this back once I get that working. There seems to be quite a lot of abstraction/structure in the code that I don't immediately understand how to use. Would someone be able to explain briefly what the intention was and what the names mean? Why "factory" and why "default_*"?

kineticsystem commented 10 months ago

The idea is to have those two repo merged into one. https://github.com/PickNikRobotics/ros2_epick_gripper https://github.com/PickNikRobotics/ros2_robotiq_gripper There is a lot of code in common in these two repos, literally the same code. We want to merge the two repos, have each gripper specific code in its own package, and all the common code in a third package. We should do that first before adding an any extra gripper.

The idea behind all those abstractions is testability. Each code layer can be tested in isolation without connecting to the hardware. This diagram may help you understand how all the abstractions are combined. https://github.com/PickNikRobotics/ros2_epick_gripper/blob/main/docs/class_diagram.png

When you see something that start by default_ it means it is the default implementation for a given interface.

kineticsystem commented 10 months ago

Unfortunately, testing a hardware interface in isolation is impossible because it is a concrete class. For this reason, I added a factory to create a driver and plug it into the hardware interface. We can mock the factory and make the hardware interface believe it is connected to hardware while it is not. The Default Driver returned by the Default Factory can also be used in isolation, without ROS, and in fact is used on its own inside some test executables in the hardware_test package. These tests are used to check id the hardware is properly configured.

If you need more information we can setup a meeting, I think it is easier to guide you through the code.

kineticsystem commented 10 months ago

One last thing. If you look at the diagram https://github.com/PickNikRobotics/ros2_epick_gripper/blob/main/docs/class_diagram.png DefaultSerial, Serial and MockSerial, they are all common code between the two grippers, and they are used to implement the serial communication.

PeterMitrano commented 10 months ago

Thanks Giovanni! So when merged, default_driver_factory would be renamed to 2f_driver then we'd also have 3f_driver and epick_driver?

Do you think merging the 2f and epick repos will happen in the next month or two? I will be working with this 3f code for a little while, so I could help get all three in the same repo.

kineticsystem commented 10 months ago

We could rename them or keep the same names and use the namespaces to distinguish them. They already live in different namespaces.

I would like to have them merged sooner rather than later; I do not think it is a difficult task.

If you follow the structure of this repo and reuse the classes for the serial communication, we can drop your code easily into the merged repo.

Hopefully, the only code that will differ will be the driver, which is specific to your hardware. It is important that your code is as testable as the existing one. We already provided tests for the serial communication; you only have to test your hardware interface and driver and provide a CLI to test the actual hardware.

If you would like me to drive through the code, let me know.

PeterMitrano commented 10 months ago

Ok, I'll base mine of the epick one then. If someone makes a PR to merge epick into this repo then please tag me so I can see how it looks

PeterMitrano commented 10 months ago

Here's my working draft, now based on the epick repo.

moriarty commented 10 months ago

Cool, Yes I think it would be great to get them all merged together. We're using the e-pick and 2f-85 and it would be nice to keep everything in one place and share the maintenance overhead.

Based on #40 and a slack msg from @eholum someone is also working on the Hand-E gripper support.

PeterMitrano commented 10 months ago

Question for you all -- how do you think I should be support "modes" and "mode changing" in ros2 control? Do I need to write a custom "controller" that provides the ROS topics/services for setting and getting modes? I was also considering just putting that directly in the driver for simplicity. I don't think the standard controllers and state/command interfaces are useful for custom modes like the robotiq 3f gripper has. How would y'all do it?

moriarty commented 10 months ago

I haven't gotten to play with the 3f gripper nor am I familiar with it's custom modes but I'd probably discuss it with @MarqRazz and on roscontrol.slack.com

PeterMitrano commented 10 months ago

Oh cool I didn't know there was such a thing -- does someone have to add me? I'm not sure how to join.

MarqRazz commented 10 months ago

I don't think the standard controllers and state/command interfaces are useful for custom modes like the robotiq 3f gripper has. How would y'all do it?

@kineticsystem how did you support the modes and additional states (like vac pressure) on the e-pick gripper? Are they runtime configurable?

PeterMitrano commented 10 months ago

Cross-posting here: https://discourse.ros.org/t/looking-for-feedback-on-design-on-ros2-control-interface-for-robotiq-3f-gripper/35122

I've got the driver working and now I'm just trying to figure out how to make it more usable and fit with ros2 control best practices. One thing I'm struggling with is creating an intuiting ROS API, since apparently one can only bind state/cmd to doubles? I used this in conjunction with the build in JointGroupPositionController, but this creates a very ugly command interface with Float64MultiArray which is really hard to understand. So instead I'm thinking I should probably write a custom controller which it seems is what the epick repo does.

kineticsystem commented 10 months ago

I don't think the standard controllers and state/command interfaces are useful for custom modes like the robotiq 3f gripper has. How would y'all do it?

@kineticsystem how did you support the modes and additional states (like vac pressure) on the e-pick gripper? Are they runtime configurable?

@MarqRazz The gripper has fondamentally two states, on and off. All the rest are parameters that are read during the initialization of the driver.

@PeterMitrano This is a question I spent a lot of time myself, and this is my answer. All joint or GPIO interfaces are float values, and there is very little difference between the two, just the meaning.

  1. If you want to represent a rotation of a joint, then obviously use a joint interface.

  2. If you want to represent a binary, you may want to use a GPIO interface, although it is still a float value, and you need to deal with the conversion between bool and float. There is no standard way to convert between the two, be careful.

  3. For enums, you probably still want to use GPIO interfaces, but the conversion between enums and float may start to be a bit on the edge. e.g. 0.0 <= x < 1.0 = enum 1 1.0 <= x < 2.0 = enum 2 etc.

  4. For everything more complex than that, you are better off adding a ROS service, publisher, subscriber, or action directly into the driver. The controller and the driver can then talk with each other using ROS messages, or your client can avoid the controller altogether and talk directly with the driver.

kineticsystem commented 10 months ago

In the ePick driver I used the first 3 options, but for a personal object I'm using the 4th and not using a controller at all.

PeterMitrano commented 10 months ago

I think having this on ROS Discourse might be more visible and useful to the community, so I'm responding to Giovanni's comment there :)

PeterMitrano commented 10 months ago

I'm seeing that the serial library is running really slowly for me, like 15 milliseconds just to send one command which is like 8 bytes. @kineticsystem @MarqRazz have y'all tested or noticed anything like this?

PeterMitrano commented 10 months ago

Actually what I'm seeing might just be that it takes the robotiq 15ms to respond with serial data... I wrote a little test with boost::asio and it also takes 15ms before read returns anything.

#include <boost/asio.hpp>

int main(int argc, const char** argv)
{
  boost::asio::io_service io;
  boost::asio::serial_port port(io, "/dev/ttyUSB1");
  port.set_option(boost::asio::serial_port_base::baud_rate(115200));

  std::vector<uint8_t> request{
    0x09, 0x10, 0x03, 0xe8, 0x00, 0x03, 0x06, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x72, 0xe1
  };

  for (int i = 0; i < 100; ++i)
  {
    boost::asio::write(port, boost::asio::buffer(request));

    size_t response_size = 8;
    std::vector<uint8_t> read_buffer(response_size);

    try
    {
      auto const t0 = std::chrono::high_resolution_clock::now();
      auto const bytes_read = boost::asio::read(port, boost::asio::buffer(read_buffer));
      if (bytes_read != response_size)
      {
        RCLCPP_ERROR_STREAM(kLogger, "Read " << bytes_read << " bytes, expected " << response_size << " bytes.");
      }
      auto const t1 = std::chrono::high_resolution_clock::now();
      auto const dt_us = std::chrono::duration_cast<std::chrono::microseconds>(t1 - t0).count();
      RCLCPP_INFO_STREAM(kLogger, "serial dt:" << dt_us << " us "
                                               << " for " << response_size << " bytes.");
    }
    catch (const boost::system::system_error& e)
    {
      RCLCPP_ERROR_STREAM(kLogger, "Error reading from serial port: " << e.what());
    }
  }
}
PeterMitrano commented 10 months ago

I'm just going to ignore the slowness of the serial comms for now, it doesn't seem to actually cause any problems. So I think I'm ready to make a PR with my new driver for the 3f.

I'd be adding the following packages:

robotiq_3f_description
robotiq_3f_driver
robotiq_3f_hardware_tests
robotiq_3f_interfaces
robotiq_3f_transmission_plugins

I think I should wait to PR until the epick repo and this one have been merged, since right now I have a ton of code duplication with the epick gripper. I'm happy to help review that merge now that I'm a bit familiar with the code!