drivers/at: fix URC collision with command response

[derMihai]

Contribution description

Fix URCs being occasionally mistaken for command response.

Causes

Some DCEs (e.g. LTE modules from uBlox) define a grace period where URCs are
guaranteed NOT to be sent as the time span between:

• the command EOL character reception AND command being internally accepted
• the EOL character of the last response line

As follows, there is an indeterminate amount of time between:

• the command EOL character being sent
• the command EOL character reception AND command being internally accepted,
  i.e. the begin of the grace period

In other words, we can get a URC (or more?) just after issueing the command
and before the first line of response. The net effect is that such URCs will
appear to be the first line of response to the last issued command.

The current solution is to skip characters that don't match the expected
response, at the expense of losing these URCs. Note, we may already lose URCs
when calling at_drain() just before any at_send_cmd(). Success partially depends
on whether command echoing is enabled or not:

1. echo enabled: by observation, it seems that the grace period begins
   BEFORE the echoed command (@aaltuntas can you please confirm this?).
   This has the advantage that we ALWAYS know what
   the first line of response must look like and so if it doesn't, then it's a
   URC. Thus, any procedure that calls at_send_cmd() will catch and discard
   these URCs.
2. echo disabled: commands that expect a response (e.g. at_send_cmd_get_resp_wait_ok())
   will catch and discard any URC (or, if MODULE_AT_URC enabled, hand it over to
   the URC callbacks). For the rest, it is the application's responsibility to
   handle it.

If missing URCs is unacceptable

As already mentioned, 100% URC coverage is already not guaranteed, but if that needs to be improved, the approach suggested by @aaltuntas here should be implemented.

Testing procedure

I added a python script that simulates some hard-coded responses, with URCs right before the response. Follow the instructions inside.

This is not tested against at_urc_isr, as it introduces some timing issues nevertheless. See #20059

Issues/PRs references

#20059
#14327

[derMihai]

Removed because it implies the command and response buffer may overlap, which by function signature would be UB.

[riot-ci]

Murdock results

✔️ PASSED

e99cafb drivers/at: fix URC collision with command response

Success	Failures	Total	Runtime
7956	0	7956	10m:06s

Artifacts

• Documentation preview

[kfessel]

The timeout might be reset multiple times. Is that ok?

[derMihai]

This is the way timeouts worked the whole time, so I stuck to this behavior.

[kfessel]

there wasn't a loop before

before:

at_expect_bytes(dev, bytes, timeout); // takes upto timeout time 

takes upto timeout time

me unroll

this:

at_expect_bytes(dev, bytes, timeout); //takes upto timeout time
if (res != 0 && res != -ETIMEDOUT) at_expect_bytes(dev, bytes, timeout);//takes upto timeout time
if (res != 0 && res != -ETIMEDOUT) at_expect_bytes(dev, bytes, timeout); //takes upto timeout time
if (res != 0 && res != -ETIMEDOUT) at_expect_bytes(dev, bytes, timeout); //takes upto timeout time
...

takes upto N-times time out unless a 0 or timeout is returned

[kfessel]

ah i see 'at_expect_bytes' would also trickle from byte to byte so the timeout is expected to be from byte to byte, not for the whole function

[derMihai]

I'm not fond of this way of handling timeouts either, but this goes all way down to how isrpipe_read_timeout() works. On the other hand, the modems usually send data in bursts, so I can hardly imagine a scenario where timeouts massively add up.

[kfessel]

thanks for clarification

[kfessel]

see the static test fail

[kfessel]

Suggested change

	* @returns line length on success
	* @returns <0 on error
	* @retval n line length on success
	* @retval <0 on error

[kfessel]

the PR seems good to me

[kfessel]

please start your PR description with

What this changes

and then the

why

( the PR description is part may be part of the merge commit )