Made reset work with hal_gpio.

[drummer315]

Made reset work with hal_gpio. I have only been using linuxcnc for about a week, so I don't know if this change is reasonable. But it works on my Raspberry Pi 5.

[andypugh]

Do you definitely need the old_vals? The use-case for reset is working with a stepgen, and in that case the step-time is set to zero. I think that this means that the input HAL pin to the hal_gpio driver pin will always return to zero on the next thread invocation, so it is safe to blindly write the HAL value to the hardware pin without edge checking.

If you look at the equivalent code in the hal_parport driver, I don't see any edge-detection there: https://github.com/LinuxCNC/linuxcnc/blob/master/src/hal/drivers/hal_parport.c

You seem to have put the test for whether the deadline has expired before the calculation of what the output pin states will be. I had it deliberately after to save cycle time (as you are then assembling the code during the reset time, rather than after it.

[drummer315]

If I understand what you are saying, I should set stepgen.steplen = 0 when using reset. Maybe that's why I could not get reset to work without my patch. There was comment saying reset wasn't implemented, so I thought it was incomplete. I don't know how stepgen worked and stepconf set steplen=1. Then logic told me that I wanted to generate a pulse starting at each transition and ending 5ms later. So that's what I did--but from what you are saying, I think it is the wrong way to do it. I will try without the patch and setting steplen=0.

[andypugh]

steplen = 1 should also be fine, that's 1 nanosecond.

[drummer315]

I haven't been able to get reset to work without the patch. But that could be because I don't know how to use linuxcnc. I think you are saying that reset works without this patch. But you haven't stated that clearly. Does reset work with hal_gpio as is in Master? When I try to use without patch, I get no errors, but it steps at same rate as without reset. With this patch, it steps at double the rate. If reset works in Master, then I will delete this pull request.

[andypugh]

I don't actually know if reset works without the patch. I did leave in the comment that reset wasn't supported, but it looks like I put in a lot of the stuff needed to make it work.

It helps to consider how real-time components work in LinuxCNC. Each component is executed every time the thread it is attached to runs, and they run strictly in the order that they are attached to the thread in.

Considering just the base thread, we would expect the relevant part of the sequence to be stepgen > write function > reset function.

At a slow step rate we would expect the stepgen to create a step (step pin high) during one thread cycle, then set it back to low on the next if the step length is less than the base thread time. If the step length is set to 1 ns then this will always be true. In this case there is no need for an edge-detector in the write function, as the input pin will only ever be high for a single thread cycle.

At the absolute maximum thread rate, where the thread cycle time is equal to the step rate, again it should work. The step output will go high, be copied into the hardware pin, then reset. On the next thread cycle, the stepgen output will again be set high (as a step is due), the value will be sent to the hardware pin, and then be reset.

Note that sometime you might put the reset function first in the thread, as this will avoid the busy-wait ever being needed. But that will only work if the execution time of the rest of the thread is longer than the space time for the driver.

It is also debatable if the reset function is particularly useful with real mechanical systems, too. The maximum step rate is one step per cycle. The next slower rate is one thread per two cycles. Can the stepper motor track that doubling in speed?

Regardless, I think that the reset function should work OK (in the LinuxCNC base thread context) without an edge detector in the write function.

[drummer315]

I must be stupid. I can't get reset to work without detecting both edges. I don't understand how hal_gpio or hal_parport communicate to stepgen that it reset the line. As far as I can tell, they do not. And if they do not, I don't see how it can work without detecting both edges. But then, I am stupid, so maybe that's why I don't understand.

It is also debatable if the reset function is particularly useful with real mechanical systems, too. The maximum step rate is one step per cycle. The next slower rate is one thread per two cycles. Can the stepper motor track that doubling in speed?

Well, this seems better than maximum step rate is 1/2 per cycle and next slower rate is 1/3 per cycle. I don't have any experience with CNC or steppers or any of this stuff, so I could be wrong. I just got a CNC machine to make some stupidly expensive and hard to find drum parts.

[drummer315]

I realize you don't need to teach a beginner like me how this stuff works. And I am thankful for your help so far. But I just can't get this thing to work without detecting both edges. I don't understand how hal_parport can work. This is what I think is happening based on my reading of stepgen.c, hal_parport.c and hal_gpiod.c. Let's assume stepping at maximum rate and reset active and no inverting.

1. stepgen.make-pulses sets stepgen.step high.
2. hal_gpio.write tells gpiod to set the physical pin high.
3. hal_gpio.reset busy waits 5000ns.
4. hal_gpio.reset tells gpiod to set the physical pin low.
5. yield for the balance of BASE_PERIOD
6. stepgen.make-pulses sets stepgen.step low.
7. hal_gpio.write tells gpiod to set the physical pin low--nothing happens because it is already low.
8. yield for the balance of BASE_PERIOD
9. start over at step 1.

Result: one step every two BASE_PERIOD.

With the patch, this is what I think is happening:

1. stepgen.make-pulses sets stepgen.step high.
2. hal_gpio.write detects edge and tells gpiod to set the physical pin high.
3. hal_gpio.reset busy waits 5000ns.
4. hal_gpio.reset tells gpiod to set the physical pin low.
5. yield for the balance of BASE_PERIOD
6. stepgen.make-pulses sets stepgen.step low.
7. hal_gpio.write detects edge and tells gpiod to set the physical pin high.
8. hal_gpio.reset busy waits 5000ns.
9. hal_gpio.reset tells gpiod to set the physical pin low.
10. yield for the balance of BASE_PERIOD
11. start over at step 1.

Result: one step every BASE_PERIOD.

I must be misunderstanding something.

[andypugh]

Firstly, I will stress that I don't know for sure that reset works for this driver. I think it should, but I didn't test it with an actual stepper motor. I think that there is a good chance that it needs fixes, but I don't think the edge detector is one of them.

In your first case, if the step rate is faster than two base periods, in phase 6, the stepgen will set the step pin high again (or, rather, leave it high, rather than setting it low) and that will then set the gpiod pin high too, and the result is a step every base period.

In your second case, in step 7, if that is what it does then you actually get twice the desired step rate, and that isn't right.

Every time the stepgen step pin goes high, we want a pulse from the GPIO pin. But only on the low-to-high (or, at the limit, still-high) transition.

Can you attach your HAL file? Possibly there is something not quite right there.

[drummer315]

I see. I misunderstood how linuxcnc works. I thought, incorrectly, with reset enabled, versus reset disabled, and no other changed to the hal or ini files, that it should step twice as fast. But based on what you are saying, it should not. It should step at the same rate. However, it should work at twice the step rate.

With my new understanding, I believe hal_gpio.reset works perfectly as is in Master. The only thing I would change is get rid of that comment saying that reset isn't supported yet. Well, except that it doesn't work if you have no input pins. Work around is to pick some random pin to use as a dummy input pin.

Thank you for your help. I am sorry for wasting your time with my ignorance.

[andypugh]

I see. I misunderstood how linuxcnc works. I thought, incorrectly, with reset enabled, versus reset disabled, and no other changed to the hal or ini files, that it should step twice as fast. But based on what you are saying, it should not. It should step at the same rate. However, it should work at twice the step rate.

Yes that's right. Perhaps the documentation needs to be clearer on this point.

The only thing I would change is get rid of that comment saying that reset isn't supported yet.

Yes, that does seem like it should be changed.

Well, except that it doesn't work if you have no input pins. Work around is to pick some random pin to use as a dummy input pin.

That sounds like an interesting bug that I was unaware of. Can you raise an issue on the tracker about this? (and assign it to me if the system lets you do that)

[andypugh]

I have removed the misleading comments: https://github.com/LinuxCNC/linuxcnc/commit/fadbc8d0ce914df90f7e8a213cca884b36f8b478