Stepper update targetting accelStepper #66

dtex · 2016-10-19T19:24:43Z

This has the changes proposed by @soundanalogous in #42 except for the analog
homing/limit option. I like the homing idea but I’d like to get this
done first (it’s already a lot)

soundanalogous · 2016-10-20T04:02:23Z

stepper.md

+6  END_SYSEX                               (0xF7)
+```
+
+**Stepper limit**


Do we also need a state (HIGH | LOW) for each limit pin? If not the protocol should state what the expected state is.

soundanalogous · 2016-10-20T04:09:23Z

stepper.md

+0  START_SYSEX                              (0xF0)
+1  Stepper Command                          (0x62)
+2  config command                           (0x20)
+3  group number                             (0-4) (supports up to 5 groups)


Can a two or more groups include the same device?

I couldn't find anything in the documentation or the forums, but I see there is nothing in the code to guard against it. I had chosen to limit it to 5 groups because in my mind with 10 steppers you could only create 5 groups, but now I see that you could create any number of combinations so I'll fix that.

BTW, I've had use cases where it made sense to include servos in multiple groups so I think it's a reasonable thing to allow for steppers.

soundanalogous · 2016-10-20T04:10:20Z

stepper.md

+0  START_SYSEX                              (0xF0)
+1  Stepper Command                          (0x62)
+2  multi to command                         (0x21)
+3  num group number                         (0-4)


change to just "group number" and on next line to "member number", etc

soundanalogous · 2016-10-20T04:11:14Z

stepper.md

+1  Stepper Command                         (0x62)
+2  stop command                            (0x06)
+3  device number                           (0-9)
+14 END_SYSEX                               (0xF7)


4 instead of 14

soundanalogous · 2016-10-20T04:13:54Z

stepper.md

@@ -1,64 +1,214 @@
 Stepper Motor


Call this Stepper Motor 2.0 and leave the old Stepper protocol, naming it Stepper Motor 1.0. I'll deprecate it once 2.0 is official but it's good to keep it around.

soundanalogous · 2016-10-20T04:16:19Z

stepper.md

+```
+
+**MultiStepper configuration**
+```


Please add a brief explanation about how this works. It took me a while to figure it out. Explain if different groups can share steppers and any other info that may not be obvious to someone reading over this for the first time.

soundanalogous · 2016-10-20T04:18:49Z

stepper.md

+14 END_SYSEX                                (0xF7)
+```
+
+**MultiStepper to**


Is it desirable or practical to also add an acceleration / deceleration option for the group?

This is not supported by multiStepper

soundanalogous · 2016-10-20T04:19:50Z

stepper.md

+1  Stepper Command                         (0x62)
+2  to command                              (0x03)
+3  device number                           (0-9)
+4  num target byte1 LSB


What is the target? Is it an angle? A number of steps, half steps, micro steps, etc? Please add a short description.

soundanalogous · 2016-10-20T04:21:39Z

stepper.md

+12 END_SYSEX                               (0xF7)
+```
+
+**Stepper enable**


Do we need separate enable and disable commands? Could this just be enable true | false. Could add one more byte for the state, or OR it with the device number byte.

soundanalogous · 2016-10-20T04:23:25Z

stepper.md

+14 END_SYSEX                                (0xF7)
+```
+
+**MultiStepper to**


Is there a particular reason for not having a MultiStepper step command?

Short answer: It's not supported by multiStepper.

Slightly longer answer: I think the primary arrangement for multiStepper is for controlling an end effector that operates on a 2D plane or in a 3D space. In these systems users need to tween between two points in a linear fashion. This would always be done in a to(X, Y) or to(X, Y, Z) fashion.

soundanalogous · 2016-10-20T04:23:40Z

stepper.md

+1  Stepper Command                         (0x62)
+2  multi stop reply command                (0x24)
+3  group  number                           (0-4)
+4  num member number                       (0-9)


member number

soundanalogous · 2016-10-20T04:26:00Z

stepper.md

+14 END_SYSEX                               (0xF7)
+```
+
+**Stepper reply (sent when a move completes or stop is called)**


Rename to something more specific such as Stepper report position or Stepper end sequence replysince there could be other types of replies in the future such as Stepper getPins, etc.

soundanalogous · 2016-10-20T04:32:05Z

stepper.md

+                                                XXXXXX0 = no enable pin
+                                                XXXXXX1 = has enable pin)
+
+5  stepDelay | interface                      (0-127)    


just stepDelay. I guess since there is a whole byte for this now, we could just spec it as the delay in microseconds with 1 microsecond as the default value. I wonder if it could alternatively be OR'd with minPulseWidth if minPulseWidth would never be close to 127.

I believe that a minimum pulse width of 63us is adequate so we could use that bit but if we ever needed more than two possible values for stepDelay it would really bork up the protocol for implementors.

soundanalogous · 2016-10-20T04:33:25Z

stepper.md

+3  device number                              (0-9) (Supports up to 10 motors)
+
+4  interface                                  (upper 3 bits = wire count:
+                                                010XXXX = two wire


I assume 2 wire is the easydriver type of stepper driver right?

Oops, that's one of 'em but there is also a two wire non-driver configuration. I'll fix that.

dtex · 2016-10-20T17:54:01Z

Switching the file name broke the link between your comments and the lines they affect.

If I didn't explicitly reply to any comment on the first commit (in stepper.md) than just know that I agreed and "made it so".

Comments with replies from me are:
#66 (comment)
#66 (comment)
#66 (review)
#66 (review)
#66 (review)

soundanalogous · 2016-10-21T04:41:40Z

stepper2.md

+                                                XXXXXX0 = no enable pin
+                                                XXXXXX1 = has enable pin)
+
+5  stepDelay                                  (0-127)


Thinking more about this... does AccelStepper have the concept of a stepDelay? If not we probably don't need it. This is something I had to hack in to my custom stepper controller in order to support a wider variety of two wire step + direction drivers but I'd suspect AccelStepper probably has a built in way to handle this.

They do not have anything in code that is equivalent to stepDelay.

Can you point me to one of those other drivers?

http://www.schmalzhaus.com/EasyDriver/

If I remember, these are the drivers that needed the delay increase from 1 microsecond to 2 microseconds: https://www.pololu.com/category/120/stepper-motor-drivers. The EasyDriver worked fine with the default 1 microsecond delay.

Looking at AccelStepper.cpp minPulseWidth serves the exact same purpose that stepDelay did in my driver class so we can safely remove stepDelay here.

dtex · 2016-10-22T16:45:50Z

Is it okay to add in the analog limit/homing features into the protocol even if I don't plan on implementing them in stepper2 right away?

soundanalogous · 2016-10-22T17:15:01Z

Sure. The entire protocol does not need to be implemented, but a comment in the header of implementation class(es) should state which parts of the protocol are not implemented.

dtex · 2016-11-01T17:10:04Z

Digging into the encoder/homing stuff for the protocol I've realized that this may be folly. There is more than one way to measure the position of a stepper motor (potentiometer, optical encoders, magnetic encoders) and to handle all of them in firmata would be bloaty. There is a round trip/delay to handle this on the host server instead but I think it's the right thing to do. If someone is so concerned with responsiveness that the encoder/homing stuff needs to be on board, then they probably aren't going to be using Firmata anyway.

I'm leaving the digital limit switches because that's a good safety feature that is easy to handle.

soundanalogous · 2016-11-02T04:50:31Z

I think the stepDelay is the last unresolved piece here. I don't think we need it since AccelStepper doesn't need it and I'd assume that lib supports a very wide variety of stepper motors. The only reason I guess to keep stepDelay would be if this protocol is generic enough to work with other libraries.

dtex · 2016-11-04T20:31:58Z

I don't think that other libraries will have enough in common with accelStepper to be a good fit as-is. Things like multiStepper, acceleration would very possibly be handled differently, if at all.

We could leave stepDelay in there and then if it turns our we need it we could just try and get a PR landed in accelStepper.

soundanalogous · 2016-11-05T17:08:30Z

stepper2.md

+5  num target byte2 MSB
+6  speed LSB                               (steps per second)
+7  speed MSB
+8  [optional] accel LSB                    (acceleration in steps/sec^2)


Looking at the AccelStepper docs, it doesn't appear to support separate acceleration and deceleration rates. Looks like you specify a single value it it's used for both acceleration and deceleration. It also appears that acceleration is something you set once for a motor for the duration the application runs rather than each time you send the step command in which case we'd need to move it to config or to it's own command.

Actually looking at some more examples, setAcceleration can be used more than once, but it's an expensive call. It may be better to decouple it from any of the step commends then and if a user wants to set acceleration for a motor, they send a SET_ACCELERATION command before sending one of the step commands. It also gives them the option to set the acceleration only once for the lifetime of the program.

I'm hopeful that a separate deceleration parameter will be added to accelStepper but for now it is just the one. Clearly they haven't seen how a nice easing function can make a robot seem less, uhm, robotic.

I agree with breaking out SET_ACCELERATION. I'll make that change.

soundanalogous · 2016-11-05T18:31:29Z

stepper2.md

+0  START_SYSEX                             (0xF0)
+1  Stepper Command                         (0x62)
+2  set acceleration command                (0x08)
+3  accel LSB                               (acceleration in steps/sec^2)


Missingdevice number

@soundanalogous

This has the changes proposed by @soundanalogous except for the analog homing/limit option. I like the homing idea but I’d like to get this done first (it’s already a lot)

soundanalogous · 2016-11-05T21:31:52Z

@dtex I think this is good to go unless you have anything else you'd still like to add.

dtex · 2016-11-05T23:39:43Z

I'm good with it. I'll start hacking on configurableFirmata to match this.

dtex changed the title ~~Initial commit~~ Stepper update targetting accelStepper Oct 19, 2016

dtex mentioned this pull request Oct 19, 2016

Proposed addition to stepper rwaldron/johnny-five#1218

Open

soundanalogous reviewed Oct 20, 2016

View reviewed changes

stepper.md

1 Stepper Command (0x62)

2 stop command (0x06)

3 device number (0-9)

14 END_SYSEX (0xF7)

Copy link

Member

soundanalogous Oct 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 instead of 14

soundanalogous reviewed Oct 20, 2016

View reviewed changes

soundanalogous reviewed Oct 21, 2016

View reviewed changes

soundanalogous added this to the v2.6 milestone Oct 22, 2016

soundanalogous mentioned this pull request Oct 22, 2016

Updates to the Stepper Protocol #24

Closed

soundanalogous reviewed Nov 5, 2016

View reviewed changes

Initial commit

3ba4ee7

This has the changes proposed by @soundanalogous except for the analog homing/limit option. I like the homing idea but I’d like to get this done first (it’s already a lot)

soundanalogous merged commit cc342de into firmata:master Nov 6, 2016

Stepper update targetting accelStepper #66

Stepper update targetting accelStepper #66

Conversation

dtex commented Oct 19, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dtex commented Oct 20, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dtex commented Oct 22, 2016

soundanalogous commented Oct 22, 2016

dtex commented Nov 1, 2016

soundanalogous commented Nov 2, 2016

dtex commented Nov 4, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dtex Nov 5, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soundanalogous commented Nov 5, 2016

dtex commented Nov 5, 2016

dtex commented Oct 20, 2016 •

edited

Loading

dtex Nov 5, 2016 •

edited

Loading