drivers/dht improvements (take 2)

[MostlyHarmless]

I created a new attempt to making the DHT driver async in this commit for review and comments. If you had checked out that branch before, some revert and rebase+squash as happened in it, so you might want to nuke your checkout and make a fresh one.

The approach in this commit is different:

• it creates a new method ameasure() that is meant to be called from a uasyncio task as

  Code: Select all

  await dhtobj.ameasure()

• it addresses the concern of @dpgeorge that the initial approach still had an 18ms blocking wait,
• moves everything that does not need to be in the critical section out of the C code into Python,
• unifies the usage of the HAL code across ports,
• works backwards compatible even if the uasyncio module is not available at all.

What had thrown me off originally was that the ports are a bit disjointed when it comes to driving a port in OPEN_DRAIN mode. Configuring the port from Python and then using the special DHT macros in the esp8266 port didn't work so well. After digging through the code a bit deeper it became clear that the abstraction layers already existed and the "special" handling of DHT IO in mp_hal_... macros wasn't really needed at all.

This implementation works with the current uasyncio as well as with the new one, that is in the pipeline.

I don't know where that "18ms" low signal is really coming from. The DHT22 datasheet says "MCU will
pull low data-bus and this process must beyond at least 1~10ms" (which is probably another Chinglish dialect for "you try, if error you try again with different waiter"). The sensors I tested here work with anything from 1ms to 30ms. Since the timing of that low phase is done with an await asyncio.sleep_ms() call, there is no guarantee that the task will be scheduled again soon enough. I think therefore it is necessary to make that duration configurable (parameter start_us in __init__(). If a particular sensor works with 2ms, then that will buy the user an additional 16ms of spare time for delays caused by other tasks.

I did not have a chance to test the stm32 port (lacking a board). I looked over the code and from all I can tell it should work. Can someone else please check that it does?

This implementation still has a blocking phase of about 4..5ms. This is unavoidable since receiving the data from the DHT module is bitbanging inside of a critical section.


Questions and comments, please, Jan

[jimmo]

This is really cool - excited to see hardware APIs getting the asyncio treatment. Thanks!

Don't have a DHT22 but I might have a DHT11 somewhere to test with.

[MostlyHarmless]

Thanks jimmo,

forgot to add some test example:

Code: Select all

# ----
# main.py for DHT async testing
# ----
import machine
import dht
import utime
import uasyncio as asyncio
import gc

async def every(ms):
    while True:
        start = utime.ticks_ms()
        await asyncio.sleep_ms(ms)
        diff = utime.ticks_diff(utime.ticks_ms(), start) - ms
        if abs(diff) > 5:
            print("every({0}): diff was {1:3d}ms".format(ms, diff))

async def print_temp():
    seq = 0
    interval = 2000
    wait_until = utime.ticks_add(utime.ticks_ms(), interval)
    while True:
        wait_ms = utime.ticks_diff(wait_until, utime.ticks_ms())
        if wait_ms > 0:
            await asyncio.sleep_ms(wait_ms)
        wait_until = utime.ticks_add(wait_until, interval)
    
        start = utime.ticks_ms()
        try:
            await d.ameasure()
            end = utime.ticks_ms()
            gc.collect()
            print("Seq: {0:06d} Temp: {1:5.1f} Humi: {2:5.1f} delay: {3:3d}ms free: {4:6d}".format(seq, d.temperature(), d.humidity(), utime.ticks_diff(end, start), gc.mem_free()))
        except Exception as ex:
            print(str(ex))
        seq += 1 
    
d = dht.DHT22(machine.Pin(14), delay_ms = 0, start_ms = 5)

loop = asyncio.get_event_loop()

loop.create_task(every(20))
loop.create_task(every(25))
loop.create_task(every(30))
loop.create_task(every(35))

loop.create_task(print_temp())
loop.run_forever()

This code is designed to create scheduling problems to demonstrate that getting data from the DHT is still reliable if there are several other, competing asyncio tasks. Things obviously get worse if you add blocking stuff into the every(ms) function.


Regards, Jan

[kevinkk525]

Since the timing of that low phase is done with an await asyncio.sleep_ms() call, there is no guarantee that the task will be scheduled again soon enough. I think therefore it is necessary to make that duration configurable (parameter start_us in __init__(). If a particular sensor works with 2ms, then that will buy the user an additional 16ms of spare time for delays caused by other tasks.

You could schedule the asyncio task without a timeout then the chances are higher that you stay within the expected timeframe, and if not you could just try again:

Code: Select all

async def wait(ms, timeout):
    st=time.ticks_ms()
    while time.ticks_diff(time.ticks_ms(),st)<ms:
        await asyncio.sleep(0) # will reschedule immediately
    if time.ticks_diff(time.ticks_ms(),st)>timeout:
        return False
    return True

So you could use it like:

Code: Select all

    async def ameasure(self):
        while True:
            if self.delay_ms > 0:
                self.pin.on()
                if not await wait(self.delay_ms,30): # maybe 30ms is a too long waiting time?
                    self.pin.off() # don't know the logic here, just reset and start again
                    await a_sleep_ms(50) 
                    continue
            self.pin.off()
            if not  await wait(self.start_ms, 30): # maybe 30ms is a too long waiting time?
                # don't know the logic here, just reset and start again
                await a_sleep_ms(50) 
                continue
        buf = self.buf
        dht_readinto(self.pin, buf)

[MostlyHarmless]

You could schedule the asyncio task without a timeout then the chances are higher that you stay within the expected timeframe, and if not you could just try again:

Thanks Kevin,

I tested this with the old uasyncio.

It does seem to improve the chances of getting scheduled "better". However, it does not eliminate the problem and since the print_temp task is now non-stop on the scheduler's ready queue, it introduces a lot of extra task switches and a bit of CPU usage in between all the other tasks, so the overall performance of everything else goes down.

At the end this scenario is an overcrowded MCU. It just has too many tasks without a well predictable scheduling pattern. Every now and then we have a storm of tasks needing to be scheduled at once, someone will have to lose. If that is the task that updates the current temperature reading every n seconds, then it will not be updated every once in a while. If measuring room temperature and humidity is that critical, it should be done by a less busy MCU, IMHO.

But thanks for the idea anyway.


Regards, Jan

[MostlyHarmless]

While two years old, Issue #2989 is precisely what is missing to address the possible lack of accuracy of uasyncio.sleep_ms() in the case at hand. The issue thread is long, but well worth reading top to bottom.

To clarify, the problem at hand is that in an asyncio scenario a large number of tasks with relaxed timing requirements, all becoming runnable at the same time, may extend the request to sleep for 18ms to a duration, where the DHT22 sensor ignores it as a start signal (something around 30-35ms). So this one, single request to sleep in the whole DHT driver has a need for accuracy and priority to avoid ETIMEDOUT (caused by the sensor not recognizing the start signal and therefore not sending a response at all).

It is certainly true that under cooperative multitasking, this code is guaranteed to fail if another coro is scheduled during the 18ms wait and then hogs the CPU without yielding for 50ms straight. No priority model and no busy looping on sleep(0) will fix a coro like that.

However, that by itself is a very weak argument against implementing any sort of priorities. Even if MicroPython cannot aim for real time unless dedicated hardware is available to guarantee it, we should at least aim for some sort of predictable, maximum response time. What I mean by that is that if I have one, single high priority task, then the delay for returning from a sleep_ms() should be no more than the longest possible, non-yielding execution of any other task, plus the time needed for task switching. So if all of my other coros can only hog the CPU for 5ms max, my sleep_ms(20) is guaranteed to return in about 20..25ms plus task switching delay. If I have a second high-priority event and serving that costs 3ms, then my possible time frame goes up to 20..28ms plus two times task switching.

And the argument that Micropython should be a subset of CPython? Sorry, I think it currently is an "extended subset". By nature it has to be because the environment is very different. My suggestion would be to make the extension of uasyncio the opposite of what Peter proposed, namely to leave the default behavior as it is and introduce methods like uasyncio.asap_sleep_ms() to schedule a high priority event. As Peter says in one of the posts, the need for those precise timings is rare and the DHT22 case at hand is an example for that. So make the need for high priority timing an explicit exception with dedicated methods.

Sorry for the long rant.


Regards, Jan

[pythoncoder]

What I mean by that is that if I have one, single high priority task, then the delay for returning from a sleep_ms() should be no more than the longest possible, non-yielding execution of any other task, plus the time needed for task switching. So if all of my other coros can only hog the CPU for 5ms max, my sleep_ms(20) is guaranteed to return in about 20..25ms plus task switching delay...

Sorry, I somehow missed this post. I assume you've seen my fast_io version of uasyncio which can do exactly that, by writing a device driver as an I/O device. I have proposed a change to the new uasyncio to support this behaviour (a change which is small and simple thanks to Damien's excellent design).

In my repo there is a MillisecTimer class which uses this mechanism to provide more accurate timing than sleep(): the timer is configured as an I/O device which is polled every time the scheduler gets to run.

I know using an unofficial fork isn't ideal, but I (and others) needed this behaviour for a long time. Issues and PR's - even bugfixes - against uasyncio were getting no attention so I saw no other option than to DIY. I'm hopeful that the new version will address this and other issues I've raised. I have no desire to continue to maintain my own uasyncio

[MostlyHarmless]

What I mean by that is that if I have one, single high priority task, then the delay for returning from a sleep_ms() should be no more than the longest possible, non-yielding execution of any other task, plus the time needed for task switching. So if all of my other coros can only hog the CPU for 5ms max, my sleep_ms(20) is guaranteed to return in about 20..25ms plus task switching delay...

Sorry, I somehow missed this post. I assume you've seen my fast_io version

To be honest, I have seen fast_io mentioned more than once, but didn't look at it yet. There is only so much one can pay attention to at the same time. Likewise there is no need for you to be sorry.

In my repo there is a MillisecTimer class which uses this mechanism to provide more accurate timing than sleep(): the timer is configured as an I/O device which is polled every time the scheduler gets to run.

Sounds like the thing I'm looking for.

I have no desire to continue to maintain my own uasyncio

Nobody does, but that is the risk of doing "git clone", is it not?


Regards, Jan

[MostlyHarmless]

Since there are no objections to this in over a month, I created a PR for it.

[tve]

Question about the overall model. I want to use other sensors, such as bme680, si7021, sht31 with asyncio. They all need their time to perform a data acquisition. Fortunately, they have a more sane i2c interface than the dht so there's no time-critical sleep. The question I have is the following: you have been using a model where the driver exports an `ameasure` method that calls async delay functions inside. In my drivers I have instead provided a `start` method that starts the data acquisition and returns the expected time it will take and then a second `read` method that reads the result assuming it's ready and returns an error if it's not. The rationale for my design is that this model lets the caller decide which form of delay function to call. Is that not preferable?