Enhancement: Questions about adding support for Stepper Phidgets about phidgets_drivers

[clalancette]

I created a Stepper class in the phidgets_api and I am modeling it after your Motor class. You added a Motors class since there are some phidgets that control multiple dc motors from one VINT port. There do not seem to be any stepper phidgets that control multiple motors from one port. Should I go ahead and create a Steppers class as well even though it is not really necessary just so it better matches the way dc motors are handled? Is the idea with these drivers that there will be one node per VINT port? An alternative might be to use a vector of serial numbers and hub ports when creating the Steppers class so that multiple steppers are controlled by one node, but perhaps just creating separate nodes, one for each stepper, is cleaner.

Generally, I prefer to only implement things that are needed. So if you look at the gyroscope class, for instance, it only supports a single gyroscope since there are no phidgets that support more than one gyroscope.

In this case, though, it looks like there is a Phidget that supports more than one stepper: https://www.phidgets.com/?tier=3&catid=23&pcid=20&prodid=1033 . Because of that I'd prefer to match the Motor/Motors split here.

I want to expose almost all of the stepper phidget functions so I have them available for my future projects. The stepper phidget API has like 30-40 functions. Should I create one topic per function or does that make a mess of the topic namespace? I could group some together so we can set things like acceleration, control mode, and current limits in one setup topic and leave setting and publishing the target position or velocity in separate topics since those will be called regularly. Do you have a preference on how such things are grouped?

30-40 topics does seem like a lot. But let's rewind a bit. Typically we use topics here to publish data coming out of the device, and another method (I'll get to that later) to set/configure things.

Let's start by looking at data you want to get from the device. Since I don't know which device you have (which is it?), I'll just pick this one at random: https://www.phidgets.com/?tier=3&catid=23&pcid=20&prodid=1033#product_tab_5

There are quite a lot of things that you can "get" there, but I think a large number of them aren't that useful. For instance, any "configuration" parameters that you can both get and set aren't really necessary to be published; you already know what you set them to via your configuration. This includes the acceleration, control mode, data interval, engaged, min/max position, rescale factor, target position, velocity limit, and min/max velocity limit.

What that leaves us with for "dynamic" things is the following:

• is moving
• position
• velocity

Following the pattern from the other devices, what I'd do here is create a custom message in phidgets_msgs that has a boolean for "is moving", a float64 for "position", and a float64 for "velocity" (somewhat similar to https://github.com/ros-drivers/phidgets_drivers/blob/8ea537570737b435f40166da5e8ffb40e0836fa7/phidgets_msgs/msg/EncoderDecimatedSpeed.msg, but with different fields). You'd then create one topic per channel of this message type, so something like /stepper00, etc (again, like how it is done in

phidgets_drivers/phidgets_motors/src/motors_ros_i.cpp

Line 92 in 8ea5375

for (int i = 0; i < n_motors; i++)

). That way you'd only end up with one topic per stepper channel.

Getting back to setting values, there are two basic ways to do this. The first way that 95% of this code uses is to take parameters at startup time and then configure the device before use (as you see in

phidgets_drivers/phidgets_motors/src/motors_ros_i.cpp

Line 60 in 8ea5375

double braking_strength = this->declare_parameter("braking_strength", 0.0);

). I'd prefer to keep that pattern for the stepper motor if at all possible. If there is something that absolutely needs to be configurable at runtime, then we can introduce a service to do that (we do that in gyroscope code, for instance, to recalibrate on the fly, here:

phidgets_drivers/phidgets_gyroscope/src/gyroscope_ros_i.cpp

Line 128 in 8ea5375

cal_srv_ = this->create_service<std_srvs::srv::Empty>(

).

Let me know if all of this makes sense to you. Happy to explain more and/or review the code once you have something.

[mintar]

Just one remark: I always try to use standard ROS messages wherever possible. For a stepper motor, sensor_msgs/JointState seems like a perfect match. It already has fields for position and velocity, and I would suggest to ignore the is_moving flag, since it can probably be easily calculated from velocity != 0.0, right? This is also how it's done in the phidgets_high_speed_encoder.

[clalancette]

@mintar Ah, very good point! I totally agree with you, ignore my comment about a custom message then.

[peterpolidoro]

Using JointState messages seems like a really good idea for feedback from the steppers. What about the control messages to the steppers? The controller can either be in position control mode or velocity control mode, so it may make sense to have separate topics for setting the target position or the velocity limit, but there may be times in position control mode when you would want to set both simultaneously. I guess if the fields in JointState are optional, then either one or both of the fields could be used in a command and the rest left empty. Is there any hesitation to using a sensor_msg in a command topic or is that done regularly? Would a control_msg/JointJog be more appropriate?

There will be times when units of meters or radians will lead to inconveniently small values, but since they are floats it does matter much. Some non-phidgets stepper controllers have handy functions for determining if a target position or velocity have been reached, similar to is_moving, to keep you from having to compare floats with an epsilon that is dependent on the scale factor. It is much easier for the controller to compare integer micro step counts, but you are right, I think we can safely ignore the is_moving flag.

[mintar]

Some non-phidgets stepper controllers have handy functions for determining if a target position or velocity have been reached, similar to is_moving, to keep you from having to compare floats with an epsilon that is dependent on the scale factor.

Couldn't the driver simply publish velocity == 0.0 whenever is_moving == false? That way, we don't need to compare floats with an epsilon, but can check for exactly 0.0.

[clalancette]

Using JointState messages seems like a really good idea for feedback from the steppers. What about the control messages to the steppers? The controller can either be in position control mode or velocity control mode, so it may make sense to have separate topics for setting the target position or the velocity limit, but there may be times in position control mode when you would want to set both simultaneously. I guess if the fields in JointState are optional, then either one or both of the fields could be used in a command and the rest left empty. Is there any hesitation to using a sensor_msg in a command topic or is that done regularly? Would a control_msg/JointJog be more appropriate?

So I have seen JointState messages used as control before. But since the velocity control/position control are mutually exclusive, that doesn't seem to really fit here. If we split this into two topics like you propose (/stepper00/velocity_control and /stepper00/position_control), then we can just use a Float64 for each and not have to worry about it.