Debugging is still activated, here and below

Maybe add a comment beside the endif.

This attribute should be added only if MODULE_AT_OOB is loaded.

And it's not documented btw

if oob->urc has zero characters (strlen would be zero). oob->urc matches buffer, because all characters are correct.

Shouldn't the length of the oob->urc be checked?

The name used by modem manufactures is URC (unsolicited result code), I think it should be renamed to at_process_urc

The user doesn't know that only the reference (of the at_oob_t) is stored, can we tell the user it must store the value?

I'm uncertain if it's good method to bypass the isrpipe mutex. I know an URC can happen at any time, but the uart_init callback to character received is connected to isrpipe_write_one which unlocks the isrpipe mutex.

The way I used this method:
A gpio_int connected to the modem ring indicator. This signals a thread wakeup. Thread claims modem mutex and calls at_process_oob. And URC is dispatched to the correct callbacks.

If an URC is sent at any time, all data should be filtered for +urc:data EOL if the URC exists as callback, it should be stored and passed when at_process_oob is called. Otherwise the URC is discarded by active operations

AFAIK, an URC cannot come at anytime (i.e. not during the process of a command).
I'm also not sure to fully understand your point, and I have the impression that your proposal would mean having 2 isrpipes.

@toonst will continue working on this topic on our side, so I guess you can keep this discussion with him ;)

I see U-Blox has a well documented AT-manual. U-Blox states "module delayes URC during command execution." Quectel and Telit don't specify this....

Example:
An AT+COPS=? command can last for 180s, a SMS can be received in this period of time. Or an incomming call present itself.

I'm not saying it should be changed, I think it can be fixed if a problem occurs.

But are we going to bypass the isrpipe mutex? By calling try_read.

My proposal would be:

int res = at_readline(dev, buf, sizeof(buf), false, (uint32_t)(10 * US_PER_SEC));
if (res == 0) {
    /* skip possible empty line */
    res = at_readline(dev, buf, sizeof(buf), false, (uint32_t)(10 * US_PER_SEC));
}
if(res > 0) {
    p = buf;
    while ((*p == '\r' || *p == '\n' || *p == ' ') && ((size_t)(p - buf) < (uint8_t)res)) {
        p++;
    }
    clist_foreach(&dev->oob_list, _check_oob, p);
}

I don't really understand the issue with 'bypassing' the isrpipe mutex. As far as I understand, this mutex is only there for synchronization and to block the caller thread if the pipe is empty.

In our case, we don't want to block if empty, so no need to wait for the mutex to be unlocked.

But as I read the isrpipe/tsrb code, I have troubles to understand how the tsrb is actually thread-safe...

To solve the 'an URC can come at any time' issue, we should process URC each time a command is sent and do as you suggested before I guess. But this make everything really complicated.

I have a use case where the isrpipe should be locked:
I've left the isrpipe out of the drawing, but let the isrpipe->mutex in.

The use case is not 100% correct. But after a ring has been received, the thread should be blocked until de URC is complete.

I'm not sure to have understood everything, but isrpipe_read first tries to read from the buffer, then lock the mutex if it needs to wait for more characters.

Then if your GSM thread has a higher priority than Main thread, it will try to read from the buffer regardless of the mutex state. So there are 2 cases:

• you can read everything right away, so you check your URC and return
• you cannot read everything and you lock the mutex, then the main thread will be scheduled first because it has locked the mutex before, so the URC (or a part of it) will go to the main thread

I have the feeling that we're trying to solve a problem without knowing if it really exists. I would start by locking the whole at_dev_t with a rmutex and consider that an URC cannot be received in the middle of a command.

@vincent-d let me try over (I will skip the URC in the middle of a command/request):

I think the at_process_urc doesn't work because isrpipe_try_read only returns the number of read bytes. The process is expecting a -1 if tsrb is dried out, but it doesn't return this option.

int res = isrpipe_try_read(&dev->isrpipe, buf + read,  sizeof(buf) - read);
if (!res) {
    return;
}

Also I'm using the ring functionality of the modem. This notifies the host something is incoming. So my thread can sleep and wake on a notify. Instead of running the 'at_process_urc` endless (as you did in your modem pr).

Between a ring indication and an URC is usually a few milliseconds, I would like a block functionality int the at_process_urc to block until a URC is received. e.g. void at_process_oob(at_dev_t *dev, uint32_t timeout).
If timeout is 0 it doesn't block, if timeout is set it's blocked until a URC is completed (with EOL).

I can make this proposal if you like. But is requires #9240 to be looked at, to check the tsrb content

Alright, I understand, finally ^^. I'll try to come up with something, you can also make a proposal if you want.

But still, I'm wondering if a lock at the at_dev_t level wouldn't make sense too.