Make use of polling functionality in 16-ev3dev kernel

[ddemidov]

Introduces the following functions:

• Motor.wait(condition),
• Motor.wait_while(state, timeout=None)
• Motor.wait_until(state, timeout=None)

Make the following possible:

m.wait_while('running')
m.wait_until('stalled')

Closes #230

[ddemidov]

Tested with 16th kernel. It turns out m.wait_until('stalled') needs to be used with care, since immediately after starting motors have both 'running' and 'stalled' in state. A sleep(0.1) before calling wait_until('stalled') resolves the issue.

[WasabiFan]

Looks good, however I think you should add a note in the documentation for these functions that there may be a momentary delay and that it should be taken into account.

[ddemidov]

I could also put a sleep(0.1) in the beginning of the 'wait()' method. What do you think?

[WasabiFan]

I could also put a sleep(0.1) in the beginning of the 'wait()' method. What do you think?

Normally, I would discount that possibility as a hack 😆. However, in this case we're working with "real-time" interfaces, in the sense that many functions are doing things which take time in the real world. So I think this is at least worth considering.

One alternative I could think of would be to add some sort of logic specifically for the "stalled" case; maybe it would require that the motor stays stalled for more than 0.1 seconds (or some other threshold) before returning from the wait. In general, stalling only momentarily isn't something you care about. That has the advantage that it only kicks in when needed and is almost always what you would want, but I could imagine cases where you wouldn't want that and it might me complicated to implement.

Or, similarly, it could enact that delay condition only if you are waiting on a state that the motor is already in when the function is called.

Neither of these are perfect, but they're all worth considering I think. We should keep in mind that simplicity and predictability are important, so maybe in the end an unconditional sleep with an enable_sleep flag defaulted to True would be the best choice.

[dwalton76]

I have some similar functions in helper.py (I'll remove these in the future infavor of the patch above). I had to jump through some small hoops to work around how the initial states are set. I forget exactly what I did and am on my phone right now but maybe take a quick look at that to see what things I had to look out for. Mindcuber is a great test case for this because if anything is off at all it will be very obvious :)

[ddemidov]

I've added hold_for=100 parameter for wait methods. How it works is when condition is met first time, it sleeps for hold_for milliseconds and rechecks the condition. Only if second check is successful, the method returns.

[ddemidov]

@dwalton76, thanks, will check helper.py!

[ddemidov]

@dwalton76, it looks like wait functions in your MotorMixin class provide a bit different functionality. You are using position attribute to check if the motor is actually running, which definitely has its uses. And the methods this PR introduces only look at state attribute.

[dwalton76]

Ah good point. We should think about what to name these so that the differences are clear. Maybe the helper.py ones could be wait_pos_* and the PR ones could be wait_state_* ?

[ddemidov]

From your experience, does wait for position works better than running motor to relative/absolute position and waiting for its state to change?

[dwalton76]

Yeah the only way I could make it 100% reliable was to use the motor position

[ddemidov]

This smells both like a possible bug in drivers and a functionality duplication. It may be confusing for the users if we provide both run_to_abs_pos()/wait_while('running') and wait_for_abs_pos(). Does the need for second means the first is broken?

[dwalton76]

One scenario I remember was the state of the motor would read running before the motor actually started moving so I started using pos to work around that. I want to say I had the same problem with stopping a motor in that the state would read stopped when the motor was in fact still moving (although not for long). There were some other problems but my memory is fuzzy.

Agreed though that we don't want multiple ways to do the same thing. I still have mindcuber assembled so I'll give it a try using the functions you are adding here and try to pinpoint the scenarios where using state alone wasn't reliable.

[dwalton76]

Maybe have it return True if the condition is met and False if we hit the timeout

[ddemidov]

Maybe have it return True if the condition is met and False if we hit the timeout

Good idea! Implemented in 30e4da4

[dlech]

I could also put a sleep(0.1) in the beginning of the 'wait()' method. What do you think?

Or you could fix the motor driver itself. It should not be reporting stalled unless it is really stalled.

[ddemidov]

Or you could fix the motor driver itself. It should not be reporting stalled unless it is really stalled.

For future reference: https://github.com/ev3dev/lego-linux-drivers/blob/master/ev3/legoev3_motor.c#L637

[ddemidov]

It looks like 'stalled' flag is set whenever more time has passed between samples (are those position clicks?) than a predefined constant, max_us_per_sample. With this in mind, it seems logical that motor is considered stalled in the beginning of the run, when it has not yet gained speed.

How would you fix this? Would simple increase of max_us_per_sample work?

[dlech]

How would you fix this? Would simple increase of max_us_per_sample work?

I think we could just increase the time before we set the stalled flag. In the code that you linked, we are setting speed = 0 and stalled = true at the same time. We could modify this to just set the speed to 0 and start a stall timer. If the speed does not change for 0.1 seconds, then set the stalled flag.

[ddemidov]

We can probably drop the hold_for parameter, since it was only introduced as a workaround for stalled misreports which should be fixed by ev3dev/lego-linux-drivers#57.

[dwalton76]

I'm working on a new rubiks cube robot and the ability to do

m.wait_until('running')
m.wait_while('running')

is golden...thank you @ddemidov for implementing this

[WasabiFan]

Do you need to do those two in sequence, or is calling just the second reliable? I'm thinking we should document how we recommend that you use these methods with a sample like this.

[dwalton76]

I think you are correct that only the wait_while() is typically needed...I'll verify tonight.

[dwalton76]

I got distracted....just verified that the m.wait_until('running') is not needed.