rust-embedded / nb

Minimal and reusable non-blocking I/O layer
Apache License 2.0
86 stars 15 forks source link

Add block_while! macro that blocks while another expression evaluates to true #18

Open kellerkindt opened 5 years ago

kellerkindt commented 5 years ago

I am using this macro derivative for an private project for a while now and decided to see whether others (like you) can make use of it as well. Basically it adds a condition to block! so that it does not block indefinitely. This allows code like this:

// ...
        let time = || info.uptime_ms();
        let timeout = time() + 500;
        let in_time = || timeout > time();

        block_while!(in_time(), tx.write(START_BYTE))
            .and(block_while!(in_time(), tx.write(0x01)))
            .and(block_while!(in_time(), tx.write(COMMAND)))
            .and(block_while!(in_time(), tx.write(0x00)))
            .and(block_while!(in_time(), tx.write(0x00)))
            .and(block_while!(in_time(), tx.write(0x00)))
            .and(block_while!(in_time(), tx.write(0x00)))
            .and(block_while!(in_time(), tx.write(0x00)))
            .and(block_while!(in_time(), tx.write(0x79)))
            .map_err(drop)
            .and_then(|_| {
                Ok([
                    block_while!(in_time(), rx.read()).map_err(drop)?,
                    block_while!(in_time(), rx.read()).map_err(drop)?,
                    block_while!(in_time(), rx.read()).map_err(drop)?,
                    block_while!(in_time(), rx.read()).map_err(drop)?,
                    block_while!(in_time(), rx.read()).map_err(drop)?,
                    block_while!(in_time(), rx.read()).map_err(drop)?,
                    block_while!(in_time(), rx.read()).map_err(drop)?,
                    block_while!(in_time(), rx.read()).map_err(drop)?,
                    block_while!(in_time(), rx.read()).map_err(drop)?,
                ])
            })
            . // ...
// ...

The operation is still blocking but in this case has a timeout for the whole operation.

eldruin commented 4 years ago

This sounds fine to me. However it has been lying around for quite a while. Does anybody else have concerns about it @rust-embedded/hal ?

therealprof commented 4 years ago

Looks mostly okay to me. I do find the "rewrapping" a bit weird.

Can't we just do:

...
            match $e {
                Err($crate::Error::Other(_)) => {
                    #[allow(unreachable_code)]
                    break $e
                },
...
eldruin commented 4 years ago

Looks mostly okay to me. I do find the "rewrapping" a bit weird.

I agree. Good point. @therealprof: $e is a Result, you probably meant:

match $e {
    Err($crate::Error::WouldBlock) => {
        if !$c {
            break Err($crate::Error::WouldBlock);
        }
    },
    Err(e) => {
        #[allow(unreachable_code)]
        break Err(e)
    },
    Ok(x) => break Ok(x),
}

@kellerkindt Would you do this change, rebase to master and add an example to the documentation?

therealprof commented 4 years ago

@therealprof: $e is a Result, you probably meant:

Nope, I meant $e, as long as you're not moving the item whatever you're matching on (if in doubt, use a reference), you can just reuse it. Not sure whether that's not possible in case due to macro hygiene or something...

eldruin commented 4 years ago

@therealprof: $e is a Result, you probably meant:

Nope, I meant $e, as long as you're not moving the item whatever you're matching on (if in doubt, use a reference), you can just reuse it. Not sure whether that's not possible in case due to macro hygiene or something...

Interesting! I assumed the matching branch would move it and thus drop it on _ but I tried it and it works just fine. Thanks! @kellerkindt please disregard my comment.