wait_for pin is unreliable

Toit

#wait_for pin is unreliable

Thread channel in help

JWood48 11/20/2022 01:05 PM

I'm trying to register my homes power usage by reading each blink from my powermeter at the house. I'm using a photodiode and each time there is a blink the pin is pulled to 0.

BLINK_DETECT_PIN.wait_for 0

The problem is that wait_for is not reliable, sometimes it catches each blink, sometimes it misses 1-3 blinks before catching one again.

A simple toit example is attached, and also the equivalent arduino code. The arduino code catches every blink! Am I doing something wrong?

bmentink 11/20/2022 07:42 PM

I would suggest shifting the print to the end, as it takes time ... my 2c

JWood48 11/20/2022 08:39 PM

Have tried it without prints, same issue :/

smolesen 11/21/2022 06:29 AM

Try to use pin.do inside a task instead, seems to work for me

JWood48 11/21/2022 07:53 AM

will try that later!

floitsch 11/21/2022 10:19 AM

If you just want to count pulses, I recommend the pulse_counter library: https://libs.toit.io/pulse_counter/library-summary

floitsch 11/21/2022 10:20 AM

That said: do you know if the missed pulses are because of timing, or do we actually miss edge changes once we are solidly listening to them?(edited)

floitsch 11/21/2022 10:22 AM

In other words, the wait_for (as well as the print ...) might take some time to set up, and during that time edge transitions might be missed. However, once the wait_for is correctly set up, then we should not miss any edge transition.
For example, if you have pulses that are once every millisecond, then it's not unlikely that you will miss some.
However, if there is one every second, then that's bad and we need to investigate.

JWood48 11/22/2022 09:18 AM

will look into the pulse counter also, have not yet testet the pin.do

JWood48 11/22/2022 09:21 AM

The power meter gives 1000 blinks pr kwh used, so depending on how much power we use it varies, but during testing there was a blink every 7 seconds where it missed every 2-3 pulses. I dont think i can use the pulse counter since the timing between pulses is important to calculate the current power usage.

floitsch 11/22/2022 09:22 AM

Thanks. I will see if I can reproduce the issue.

floitsch 11/22/2022 09:23 AM

Are you running on the latest Jaguar release?

JWood48 11/22/2022 09:34 AM

Version: v1.7.5
SDK version: v2.0.0-alpha.37
Build date: 2022-11-02T13:01:57Z

floitsch 11/22/2022 09:35 AM

Do you have an idea of how long the pulses are? (So I can reproduce more easily)

floitsch 11/22/2022 09:37 AM

And when the program failed to see the blink, would it simply ignore the whole pulse or would it get stuck on the waiting to go dark phase?

JWood48 11/22/2022 10:12 AM

Just ran tests with the above examples, pin.do has same issue which is expected since it usew wait_for under the hood

JWood48 11/22/2022 10:13 AM

Interestingly the counter implementation seems to work, in a loop with 5 ms sleep i check if the counter has been increased and it catches every pulse!

floitsch 11/22/2022 10:14 AM

The counter is hardware based.

JWood48 11/22/2022 10:15 AM

So it seems there is an issue with wait_for... The blink/pulse duration is about 20ms probably a bit lower

floitsch 11/22/2022 10:15 AM

I recently "improved" the wait_for, but apparently I made a mistake.

floitsch 11/22/2022 10:15 AM

Even added tests...

floitsch 11/22/2022 10:16 AM

Investigating now what the difference could be

JWood48 11/22/2022 10:18 AM

Test file used

floitsch 11/22/2022 10:20 AM

Thanks.
Here is my test: https://github.com/toitlang/toit/blob/master/tests/esp32/wait_for1.toit

toit/wait_for1.toit at master · toitlang/toit

Program your microcontrollers in a fast and robust high-level language. - toit/wait_for1.toit at master · toitlang/toit

floitsch 11/22/2022 10:20 AM

The pulses there aren't strictly timed.

JWood48 11/22/2022 10:21 AM

Output from counter test:

DETECTED: 103 at: 1064443 elapsed: 1600
DETECTED: 120 at: 1066043 elapsed: 1599
DETECTED: 162 at: 1067633 elapsed: 1590
DETECTED: 177 at: 1069223 elapsed: 1589
DETECTED: 188 at: 1070813 elapsed: 1590
DETECTED: 204 at: 1072413 elapsed: 1599
DETECTED: 220 at: 1074013 elapsed: 1600

floitsch 11/22/2022 10:21 AM

Will try now with 20ms pulses.

JWood48 11/22/2022 10:21 AM

The counter seems to get many pulses at a time

JWood48 11/22/2022 10:22 AM

The do test:

DETECTED: 0 at: 1104575 elapsed: 1608
DETECTED: 1 at: 1104576 elapsed: 1
DETECTED: 0 at: 1106184 elapsed: 1607
DETECTED: 1 at: 1106185 elapsed: 1
DETECTED: 0 at: 1114224 elapsed: 8038
DETECTED: 1 at: 1114226 elapsed: 1
DETECTED: 0 at: 1119034 elapsed: 4808
DETECTED: 1 at: 1119036 elapsed: 1
DETECTED: 0 at: 1128674 elapsed: 9638
DETECTED: 1 at: 1128675 elapsed: 1
DETECTED: 0 at: 1135114 elapsed: 6438
DETECTED: 1 at: 1135116 elapsed: 1
DETECTED: 0 at: 1138334 elapsed: 3218

floitsch 11/22/2022 10:24 AM

The pulse_counter might be too sensitive.

floitsch 11/22/2022 10:24 AM

I think there is an option to tell it to ignore pulses that are too short.

floitsch 11/22/2022 10:26 AM

hmm. Can't find it.

floitsch 11/22/2022 10:26 AM

nvm. It's on the constructor of the Unit

floitsch 11/22/2022 10:26 AM

the glitch_filter_ns.

floitsch 11/22/2022 10:46 AM

I just added tests for 5ms pulses and those seem to work too.

floitsch 11/22/2022 10:48 AM

I'm wondering, though.
If the pulse-counter sees more pulses, then the transition might be bouncy.

floitsch 11/22/2022 10:49 AM

I didn't see it right now, but maybe the wait_for code looks at the current state of the pin when it wakes up from the interrupt, and ignores the wake-up if the pin value isn't what it expected.

floitsch 11/22/2022 10:50 AM

You could try to add a capacitor together with a pull-up/down to debounce the signal.

floitsch 11/22/2022 10:51 AM

I will continue looking in the code if that could be the reason.

JWood48 11/22/2022 10:52 AM

is wait_for significantly different than the interrupt code from the arduino sketch?

floitsch 11/22/2022 10:54 AM

I'm not that familiar with the Arduino sketch, but from the code it looks like the blink function is run in an interrupt.

floitsch 11/22/2022 10:54 AM

-> It must be fast and short.

floitsch 11/22/2022 10:54 AM

Internally, we should be doing something similar, but since wait_for is blocking your task, we have to schedule the event, so that the task gets woken up again.

floitsch 11/22/2022 10:55 AM

So there is more work to do.

floitsch 11/22/2022 10:55 AM

In the Arduino sketch, the counter is just incremented.

floitsch 11/22/2022 10:55 AM

In Toit, the event is recorded, and put into a queue, so that the task can be woken up and react to it.

floitsch 11/22/2022 10:57 AM

Looking at your code again, I'm actually surprised that it works.

floitsch 11/22/2022 10:57 AM

I would have expected that you can't printf in an interrupt function.

floitsch 11/22/2022 10:58 AM

Maybe serial.print is safe, though.

floitsch 11/22/2022 10:59 AM

But these are the kind of things we avoid by having an event system. There isn't code that is special and must be careful not to do certain operations.
For example, you would not want to run the garbage collector inside an interrupt handler.

JWood48 11/22/2022 11:03 AM

ok, will try the capacitor trick later this week probably, i'm not that much into low level stuff coming from a Java background

if that fails i will rely on the counter instead and polling in a loop...

floitsch 11/22/2022 11:04 AM

If you are curious, you could also try to see what the RMT peripheral returns.

floitsch 11/22/2022 11:05 AM

That one can measure extremely short changes.

floitsch 11/22/2022 11:05 AM

It could confirm the bouncy transitions.

JWood48 11/22/2022 11:19 AM

how would i use the rmt?

floitsch 11/22/2022 11:22 AM

I would need to test, but:

import gpio
import rmt

main:
  pin := gpio.Pin ...
  channel := rmt.Channel pin --input --idle_threshold=32000 --no-enable_filter --memory_block_count=8
  signals := channel.read
  print signals

(edited)

floitsch 11/22/2022 11:23 AM

In that configuration the RMT would consider a signal to be idle when nothing happens for 32ms.

floitsch 11/22/2022 11:24 AM

This is mostly from memory. Can't guarantee that it will work.

floitsch 11/22/2022 11:27 AM

The RMT is designed to read infra-red signals (remote controls).

floitsch 11/22/2022 11:27 AM

It waits for something to happens and records the levels, until the line goes stable/idle again.

floitsch 11/22/2022 11:28 AM

It then gives you the recorded transitions.

floitsch 11/22/2022 11:29 AM

With a pulse every few seconds we would expect to get a clean "transition from 1 to 0; 20ms; transition from 0 to 1".
However, if the input signal is bouncy, then the RMT would record something like "transition from 1 to 0; 1us, transition from 0 to 1; 2us, ....".

floitsch 11/22/2022 11:29 AM

Note that the RMT is configured for 1us here. If the transitions are faster than that, it would not see them.

floitsch 11/22/2022 11:30 AM

There is a way to make it even more precise, but then we need to be careful that the idle_threshold still fits.

floitsch 11/22/2022 11:30 AM

That said, it would probably still be enough to record a transition from 0 to 1 (especially if it's bouncy).

JWood48 11/22/2022 11:47 AM

thanks

floitsch 11/22/2022 05:27 PM

I have now rewritten the wait_for. Hopefully it will be more stable with the new approach.
Will clean up my patch tomorrow and upload it for review.
It will hopefully be in the next release.

JWood48 11/23/2022 07:02 AM

great, looking forward to test it!

JWood48 11/23/2022 07:03 AM

I just did the rmt test and get the following output:

READ_CHANNEL...
SIGNALS: 0-2590 1-1 0-1 1-1 0-1 1-1 0-1 1-1 0-1 1-1 0-1 1-1 0-1 1-1 0-1 1-1 0-1 1-1 0-1 1-1 0-1 1-1 0-1 1-1 0-1 1-2 0-1 1-1 0-1 1-0
READ_CHANNEL...
SIGNALS: 0-2580 1-1 0-1 1-1 0-10 1-1 0-1 1-1 0-1 1-1 0-1 1-1 0-1 1-1 0-1 1-0
READ_CHANNEL...
SIGNALS: 0-2588 1-1 0-1 1-1 0-1 1-1 0-1 1-1 0-1 1-1 0-1 1-1 0-1 1-1 0-1 1-1 0-1 1-1 0-1 1-1 0-1 1-1 0-1 1-1 0-1 1-1 0-1 1-1 0-1 1-1 0-1 1-0

JWood48 11/23/2022 07:04 AM

So it seems the signal is bouncy...

floitsch 11/23/2022 07:45 AM

That explains why the pulse counter sees many transitions.
However, the initial transition seems to be quite stable and should be detected by the wait_for.
If I interpret this correctly it's relatively short though: 2.5ms
This is exactly what my patch should fix.

bitphlipphar 11/26/2022 12:45 PM

Florian's patch for pin.wait_for is part of Toit v2.0.0-alpha.42 that ships in Jaguar v1.7.9, which is released everywhere except on Windows where we are waiting a day or two for a code signing issue. Almost there

bitphlipphar 11/26/2022 08:46 PM

Jaguar v1.7.9 is out on Windows.

JWood48 11/26/2022 10:15 PM

Just tested and so far it catches every pulse! thanks for the quick fix!(edited)

floitsch 11/26/2022 10:15 PM

Great news.

JWood48 11/26/2022 10:15 PM

My energy monitor is back on track

bmentink 11/27/2022 01:02 AM

@floitsch

I notice the S2/3 firmware downloaded with the latest 1.7.9 How do I flash for the S2, jag flash knows nothing about the --chip argument ... cherrs

bitphlipphar 11/27/2022 07:00 AM

@bmentink

and @floitsch: We're still missing something like https://github.com/toitlang/jaguar/pull/308.

Build software better, together

GitHub is where people build software. More than 94 million people use GitHub to discover, fork, and contribute to over 330 million projects.

78 messages in total