evil-mad / EggBot

Software for The Original EggBot
GNU General Public License v3.0
285 stars 138 forks source link

Add physical EStop feature to EBB firmware #120

Closed EmbeddedMan closed 12 months ago

EmbeddedMan commented 5 years ago

With the current (v2.6.3) EBB firmware a press on the PRG button (or RB0 going low, if that feature is turned on) will 'pause' the current plot because the PC software is constantly querying the EBB with the "QB" command to see if the user pressed the PRG button (or RB went low), and if so, it will (eventually) stop feeding moves to the EBB.

While this method has some advantages (namely that the PC completely controls the pausing function, thus allowing for easy resuming after the pause is over) the big disadvantage is that if there are several long moves in the queue (not just EBB queue but queued up on the PC side of the USB connection) then it can be a while before the pause action actually takes effect.

This feature request is to add a new command called EE that "enables estop". An optional parameter is given in this command. That parameter is the number of milliseconds after the EStop until the motors are turned off (all 3 - two steppers get disabled and RC servo output is killed). This parameter can be set to zero to disable the limping of the three motors, or it can be left off (which will then not change whatever the current limp time is). It will default to 0ms, so that if you just send an "EE" command after boot, it will enable the EStop feature but the motors will never limp.

We need a way to turn off this new feature - we could use "EE,-1" for example.

There already is an "ES" (EStop) command which can be sent from the PC to immediately halt the motors. This command does not limp the motors, and unlike the EE command being discussed here, does not stop a servo motion that is currently executing.

Do we need to add a way to query the EBB to see if the enhanced EStop feature is currently enabled, and if so, what the limp time is? Maybe instead of EE, we could piggyback on the SC command structure for this?

@oskay Looking for your input

EmbeddedMan commented 5 years ago

Would it be better to update the ES command to do the exact same thing as RB0 and PRG (with EE enabled)? I.e. if you have EE turned on, then ES command, RB0 going low or PRG being pressed would all:

If all three do the same thing, we can use common code for them and it will be easier to understand.

oskay commented 5 years ago

I infer that "limping" in this context is time after the signal is received, while the motors are still powered on.

Let me explain a little more about the problem that I'm trying to solve.

We'd like to build an enclosure around an EBB-driven device. The device will have an e-stop button and the enclosure door will have a sensor to indicate when it is closed.

It may be sufficient to have them tied together, and one input to stop the device.

OTOH, it may be better to have these as two separate inputs: E-stop which stops and then turns off power (or maybe just cuts power immediately) and a "door open" input that halts immediately but does not lose power.

Ideally, in the "door open" case, we could detect the door open, halt moving immediately and then (with additional commands) figure out where we are and resume movement if appropriate.

EmbeddedMan commented 5 years ago

Yes, correct. I was thinking the limp delay would be the amount of time before killing power to the motors.

So you're looking for two separate input actions - one which would kill power to the motors, and one which wouldn't. I don't see why that would be a problem - we just need to figure out what I/O pins would be best for each.

Now, the question of how to accurately resume where you left off is a bit trickier. If you are in the middle of a move (which you almost always are) when the EStop input goes low in, the EBB would need to figure out how much is left of the current move, stop the move, then when some new command to resume is sent, pick up where it left off in the current move. That doesn't sound too difficult, but it's different than completely dropping the current move and the move that's in the FIFO (which is what the current ES command does for example).

So, how about this as a proposal:

We define three types of stop/pause input types -

Type A : An input is simply 'remembered' if it gets set low, and the QB command can be used by the PC to see if any of the Type A inputs have gone low since the last QB command. No motor actions are taken - it's up to the PC to figure out when to pause/stop sending commands. This is how the PRG button and B0 (if enabled) input work by default at boot right now.

Type B : An input is treated as a 'pause' if it goes low. This immediately stops the current move (both servo and stepper), but remembers where it left off. Any commands in the motion queue are not deleted. The motors are not limped (they remain powered).

Type C: An input is treated as an 'EStop' if it goes low. This immediately stops the current move (both servo and stepper), and all moves currently executing and in the FIFO are deleted. If the limp-timeout is set to a non-zero value, then limp-timeout milliseconds later the motors (steppers and servo) will be de-powered.

We can set up two port B input pins (say B0 and B4) along with the PRG button as potential 'pause/stop' inputs, and use the EE command to select which type of pause/stop input each is.

Say you wanted to make the PRG button a Type A, B0 a Type B and B4 a type C with a 10ms limp-timeout. Maybe you could do

EE,PRG,A EE,B0,B EE,B4,C,10

But what happens if B0 goes low. The motors stop, fine. But how does the EBB send this fact up to the PC? Because the motion FIFO is (likely) full, no more commands can be sent from the PC to query the EBB's state. So we'd have to add some type of asynchronous message that the EBB would send to the PC in this case, and the PC would have to always be listening for this message to come at any time (that motion is happening). That's a pretty big change from how things are now on the PC side.

If B4 (when set to Type C input) goes low, we don't have the same problem because the motion queue and currently executing motion command are deleted, so the PC can always then send more commands to the EBB to query the state. We'd just have to add some more bits to the QC command maybe.

Thoughts?

hamoid commented 5 years ago

I don't know if this discussion affects the Axidraw, but in case it does: with some papers if you stop the pen and leave it there it would create a big ink spot. I mention this because in the discussion above I didn't read about lifting the pen.

oskay commented 5 years ago

Yes, I very much like this general direction.

I do not like the idea of an async message from the EBB in case 'B' That could mess up all kinds of things.

What if, instead, we set bit 5 of QG any time that any pause or stop has been detected, and wait for the computer to then ask what the situation is? Perhaps with another status byte that indicates which pin was detected. To the extent that the EBB remembers where it was paused, we should be able to send a command to resume as well. That might make it possible to have a workflow like:

(1) Move in progress (2) Type-B input signal received (3) Motion pauses (all 3 axes) (4) Computer queries QG before motion move was scheduled to end; EBB reports bit 5 is set. (5) Computer queries inputs, find that I/O pin is no longer signaling to stop. (6) Computer sends resume command, which resumes the move. (7) Planned motion continues as usual.

oskay commented 5 years ago

@hamoid The AxiDraw software uses method "A" as described above, and does lift the pen.

EmbeddedMan commented 5 years ago

@oskay That looks like a good plan to me. That's what we'll go with.

My only concern is that if you're streaming SM commands to the EBB, at step (4) you won't be able to get a QG command to the EBB because the motion queue will have a command in it and there will be another motion command waiting to be processed and the USB connection will stall because EBB won't be accepting any more bytes at that time.

However, as long as you intersperse each motion command with a QG, you'll be fine I think.

This may prompt us to implement a much larger motion queue FIFO (maybe 10 or 20 elements - whatever we have RAM for) and then let the PC do software based flow control where it's constantly querying the EBB asking how full the FIFO is and then dribbling new motion commands to it only when it has room. That would leave the USB connection 'free' for all of the query commands (like QG) which return almost instantly.


Thinking some more about this, you could use this new estop/pause as method of homing, with just a little involvement from the PC. Set up B0 as X home switch and B4 as Y home, then move one axis at a time and set both inputs up to be Type C stop/pause with no limp-timeout.


Besides changing the way QG works as you propose above, we will want a way for the PC to unpause a paused EBB (Type B input), and a way for the PC to take a paused EBB (Type B input) and delete any queued motion commands such that the next new motion command is executed after the unpause command is sent. This should give the PC enough control to get out of whatever situation arises.

oskay commented 5 years ago

This may prompt us to implement a much larger motion queue FIFO (maybe 10 or 20 elements - whatever we have RAM for) and then let the PC do software based flow control where it's constantly querying the EBB asking how full the FIFO is and then dribbling new motion commands to it only when it has room. That would leave the USB connection 'free' for all of the query commands (like QG) which return almost instantly.

That's a reasonable approach. We might need a query command with multiple-byte response that gives switch input data, QG type data, and the depth of the queue.

EmbeddedMan commented 5 years ago

Yes, that's perfect. OK, so would it be better to change the QG command to return additional information, or keep it the same (for backward compatibility sake) and make a new query command that would include a lot more info (like queue depth)?

On Thu, Jul 25, 2019 at 11:15 AM Windell Oskay notifications@github.com wrote:

This may prompt us to implement a much larger motion queue FIFO (maybe 10 or 20 elements - whatever we have RAM for) and then let the PC do software based flow control where it's constantly querying the EBB asking how full the FIFO is and then dribbling new motion commands to it only when it has room. That would leave the USB connection 'free' for all of the query commands (like QG) which return almost instantly.

That's a reasonable approach. We might need a query command with multiple-byte response that gives switch input data, QG type data, and the depth of the queue.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/evil-mad/EggBot/issues/120?email_source=notifications&email_token=AADN4CBJNLBSKY7V5F5KJPLQBHGR3A5CNFSM4IGLAJIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2Z65BA#issuecomment-515108484, or mute the thread https://github.com/notifications/unsubscribe-auth/AADN4CGSCK6DPRJ3AQ3ITTLQBHGR3ANCNFSM4IGLAJIA .

oskay commented 5 years ago

Let's keep the existing QG. I like that it's an efficient single byte.

But... How about giving QG an an optional argument? Perhaps QG[,QueryLevel]<CR>.

Then:

The second status byte could have:

Now those "watched inputs": Those could be (for example) the fixed set of PRG, B0, B2, B3, or it could omit PRG, since that's somewhat redundant-- handled in the first byte. The latter would only be watched if indicated to do so by the EE command.

We may want the QG,2 query to clear the watched input flags (if possible -- it should not be cleared if the input that caused the flag is current asserted), much like we do with the QB command.

A nice feature of doing this by expanding QG with an optional numeric argument is that it leaves room for growth -- one can imagine a QG,3 command that returns three bytes of data.

Alternate idea: We could expand QB instead (or as well). However, that is not a great place to put a FIFO depth readout.

Aside: My impression from the description of the QG query is that both QG and QB clear the "button flag", however it looks like the QB description does not mention that the flag can be cleared by QG.

EmbeddedMan commented 5 years ago

Very nice. Instead of QueryLevel being simply a number of bytes, I'd rather have it be considered like a verbosity command. The reason is because I'd like to not limit ourselves to a size of 16 for the FIFO (thinking ahead to 32-bit processors with gobs of RAM).

QG (or QG,0)just does what it always has done QG,1 returns two values. The first is just like what QG,0 does. The second is a 16 bit number which is a bit field with bits 0 through 7 representing the 'has seen a low since last QG command' for RB0 through RB7, and bit 8 being the same thing for PRG, and bits 9 through 15 being left for future needs. QG,2 returns three values, the first two are the same as QG,1, and the third is a 16 bit value which is the number of filled elements inside the FIFO. (This means that the PC will need to know how big the FIFO is in order to know if the FIFO is almost full or not, which it can determine by looking at the EBB software version number.)

This would mean that we have some room to grow without changing the meaning of the first three values returned by QG.

Also - yes, I'll update QB's documentation as well. Good catch.

oskay commented 5 years ago

Related idea to think about: Perhaps the FIFO depth could be something that the board could report in some way.

EmbeddedMan commented 5 years ago

Agreed. How about default bootup behavior is to use a FIFO depth of 1, and then there would be a new command that would allow the PC to ask "how big of a FIFO can you support?" and then another command that would say "OK, please change your FIFO size to XX". This could be done when you first connect to the EBB and would prevent newer firmware versions which support larger FIFOs from changing the behavior with older PC software that might not know how to deal with the larger FIFOs.

oskay commented 5 years ago

That's a very clever approach!

oskay commented 5 years ago

Side-note: To reduce proliferation of commands, it may be best to tack that on as a configuration option to some other command. SC and EM, for example, already support a number of different things.

One way to do approach this command would be if the new command returns fifo_depth,max_depth<NL><CR>OK<CR><NL> -- indicating both how deep it is set currently and what the maximum depth is. That way, if you try setting it to an invalid depth, it will tell you what depth it actually set it to, and what the maximum is. You would not need a separate command/query for the two.

EmbeddedMan commented 5 years ago

How about: CU,3,0 will return fifo_depth,max_depth CU,3, will set the fifo depth (to a value that is non-zero) CU,X where X is 4 through 13, will return the input type for X (X=4 means RB0, X=5 means RB1, etc. X=13 means PRG button) (return value of 0 (pause/stop disabled), 1, 2 or 3 for type A, type B or type C input) CU,X,1 sets input X to be type A pause/stop CU,X,2 sets input X to be type B pause/stop CU,X,3 sets input X to be type C pause/stop CU,X,0 turns off pause/stop for input X

QG with no parameters does exactly what it does now. QG,1 returns two values, the original byte and a new 16 bit word that contains a bit field for each of the 9 possible input bits (RB0-RB7 + PRG) - in this case, status means "has it gone low since the last QG,1 or QG,2 command). Sending QG,1 or QG,2 clears these status bits. QG,2 returns three values, the first two same as QG,1 and the third is a 16 bit value telling how full the FIFO currently is.

I think this means that you could bump the FIFO size up, then send a bunch of motor motion commands, then send QG,2 over and over and only send more motion commands when the FIFO gets empty enough. Software flow control.

I think the only thing left is controlling the pause/play state of the EBB. The only time that the EBB will enter a true 'pause' state is if a Type B pause/stop input gets activated. At that point, we need a bit that can tell the PC that the EBB is now paused. How about bit 15 of the second returned value from QG,2 or QG,3?

Then we need a command that allows the PC to tell the EBB to resume from a paused state. CU,14 maybe?

The last thing I think the PC will want to be able to do is clear out all of the motion commands in the queue and any currently playing command. Maybe CU,15 could do that. If the EBB gets paused with a bunch of commands in its FIFO, and the PC decides it just wants to re-home and start over, it will need to be able to clear out all pending motion commands.

So, in the above, I've reused QG and CU a lot for all of this stuff. Too awkward? Does it really matter (all of these commands are 'hidden' in the PC app anyway - it's probably not super important what they're actually called). Am I missing anything?

oskay commented 5 years ago

I agree with the approach in general.

One minor point, though:

I'm not keen on the idea of overlapping the Estop/pause/resume functionality and the FIFO depth control in the same command; these seem like very different types of things.

EmbeddedMan commented 5 years ago

Good point. We'll make a new command, as discussed above ("EE") for all estop/pause/resume functionality. FIFO control seems appropriate as a CU type command.

EmbeddedMan commented 5 years ago

Progress is being made! Currently code to handle the larger FIFO is going in and being tested.

EmbeddedMan commented 5 years ago

Good news, the larger FIFO code is working (finally). Next steps: add in the above commands. Slowly, slowly, it will get done.

EmbeddedMan commented 4 years ago

A partial solution to this issue has now been committed on the EBB_EStop branch. It only addresses the first part of the work to resolve this issue - the larger FIFO support. At boot, all behavior is the same as previous code (FIFO size = 1). But you can now use the CU,3,x command to increase the size of the FIFO. CU,3,0 will return the largest size that the FIFO can be. CU,3,x will set a new size, between 1 and the largest size that the FIFO can be. QG still operates as before. But QG,2 now returns exactly what QG did before but also contains the number of motion commands currently in the FIFO (which for accounting purposes also includes any executing command). Due to RAM limitations, the FIFO is currently limited to a maximum of 25 elements. As long as there is some room left in the FIFO, immediately executing commands like QG will always be executed, so it's now possible for the PC to manage handshaking at an application level rather than relying upon the USB pipe to become full and stalling the pipe at the OS level. This update to the FIFO will be used to support issue #101 (3D axis support).

At a later time the plan is to return to this issue and add EStop support as described above.

EmbeddedMan commented 12 months ago

The new Limit Switch feature (CU,51 and CU,52 and CU,53) may provide the functionality that we are looking for here. The latest code in the EBF_v3.0.0 branch also has the ability to expand the FIFO size up to 32 commands, so that part of this issue is done. QU,3 allows reading out the number of commands currently in the FIFO.

The ES command, as it stands now, is non functional. It's easy enough to stop all motion and clear out the FIFIO (no matter how large it is) as the Limit Switch feature currently does. The problem is the returned values from ES. I'm not sure it's worth it to develop code that will walk through the FIFO, and compute 'how many steps are left' for each axis in the interrupted move + all other moves in the FIFO. Maybe that's not of any use anymore.

We could simply drop the ES command. Or we could modify it to not return anything except the "interrupted" value.

Maybe what I'm asking here is to take into account the functionality currently in the v3.0.0 branch (like Limit Switch feature, FIFO size control) and see if there is still an unmet need that requires a new command, or the modification of an existing command to support that need. I think that the new Limit Switch feature may completely suffice for the original need, but I want to be absolutely certain of that before considering this issue resolved.

Any thoughts @oskay ?

oskay commented 12 months ago

I agree that there's less need for a physical interface, as this issue was originally for. I think we can close it.

But, can you clarify in what way ES is nonfuctional? Is it just because of the FIFO depth? Perhaps we could just have it report the first FIFO item (as it does now), even if the depth is > 1. Would that let it persist, at least until we can deprecate then remove it?

EmbeddedMan commented 12 months ago

Nonfunctional because I've commented out the code for it, thinking I'd need to write a function to walk through the FIFO and somehow figure out how many steps are 'left to go'.

We can absolutely put it back in, but remove the returned values except for the first one ('interrupted'). That way you have a quick way to stop all motion and flush the FIFO, without needing to do a bunch more coding.

oskay commented 12 months ago

Sounds good.

EmbeddedMan commented 12 months ago

OK, v3.0.0-a18 in the EBF_v3.0.0 branch now has a working ES command as described above. Docs have been updated as well. I don't think any further work needs to be done on this issue. It will be part of the v3.0.0 release.