xbee: add timeout for AT command response (fixes #4731)

[Yonezawa-T2]

Adds timeout for receiving AT command response.

Tested on a native platform with a XBee connected via USB adapter.
It works fine for normal case without producing debug output "xbee: response timeout". When the following lines in _rx_cb is commented out to simulate broken UART, it produces debug output "xbee: response timeout" and proceeds.

dev->resp_buf[dev->resp_count++] = (uint8_t)c;
if (dev->resp_count == dev->resp_limit) {
    /* here we ignore the checksum to prevent deadlocks */
    mutex_unlock(&(dev->resp_lock));
    dev->int_state = XBEE_INT_STATE_IDLE;
}

[Yonezawa-T2]

The test is done in a branch with #4445 merged.

[OlegHahm]

I can test next week, if needed.

[OlegHahm]

#4443 is also required, right?

[Yonezawa-T2]

@OlegHahm Thank you for testing. Yes, you need #4443 for native platform.

My experiment branch is here: https://github.com/Yonezawa-T2/RIOT/tree/xbee_border_router. This contains #4443, #4445, #4448 and some other minor patches.

Steps to test:

• connect a XBee module to Mac
• go to examples/gnrc_xbee_border_router
• edit xbee_params.h for baud rate
• make
• sudo ./bin/native/gnrc_xbee_border_router.elf tap0 -c /dev/tty.usbserial-XXXXXXXX where /dev/tty.usbserial-XXXXXXXX is the TTY device file for the XBee deivce.

The XBee driver initializes the XBee hardware on boot. On successful startup, it outputs debug outputs like following:

xbee: AT_CMD: +++
xbee: AT_CMD: ATMM2
xbee: AT_CMD: ATAP1
xbee: AT_CMD: ATAC
xbee: AT_CMD: ATCN
xbee: AT_CMD: SH
xbee: AT_CMD: SL
xbee: AT_CMD: MYZ8
xbee: AT_CMD: CH
xbee: AT_CMD: ID
xbee: Initialization successful

If we comment out some codes in _rx_cb, we will get the following outputs:

xbee: AT_CMD: +++
xbee: AT_CMD: ATMM2
xbee: AT_CMD: ATAP1
xbee: AT_CMD: ATAC
xbee: AT_CMD: ATCN
xbee: AT_CMD: SH
xbee: response timeout
xbee: AT_CMD: MY
xbee: response timeout
xbee: AT_CMD: CH
xbee: response timeout
xbee: AT_CMD: ID
xbee: response timeout
xbee: Initialization successful

As xbee.c don't check status of AT commands on startup, it shows "Initialization successful" despite of timeout.

[Yonezawa-T2]

Added status checking on startup: #4749.

[kYc0o]

ping @Yonezawa-T2

[Yonezawa-T2]

I'm waiting for review.

[OlegHahm]

Let's try to review and test this tonight.

[haukepetersen]

I think we might simplify this PR to some extend (and save memory with this): The paradigm we use in device drivers concerning broken connections is, that we check the correct connection to a device once during initialization and assume a working connection from this point on. For the xbee device, I think we should simply send a single AT comman AT once in the beginning and watch (with timeout) for the OK reply. If this times out, we abort the initialization returning an error code. If this is successful, we assume a correct connection to the device and there is no need to keep the timer struct in the device descriptor.

[Yonezawa-T2]

Moved the timer from xbee_t to the stack. The thread blocks until a response arrives or the timer expired, so we can allocate the timer on the stack. I think checking timeout only on the startup would not yield simplicity anymore.

[haukepetersen]

But it would enable the init function to return an error code. Alternatively we should add a return value to the _api_at_cmd function that we can check during init. This way init does not return a success value when it actually failed...

[Yonezawa-T2]

_api_at_cmd already returns error via resp->status and xbee_init returns error when the timer timed out (#4749).

[haukepetersen]

I don't see it. Say the call _at_cmd(dev, "ATMM2\r"); fails, the init sequence will just continue with the rest of the initialization sequence, until one of the non-AT calls later on might fail. This is not particular clean?!

[Yonezawa-T2]

xbee_init calls _get_addr_long, which calls _api_at_cmd. So connection error is certainly reported as an error from xbee_init (see #4749).

To check response for _at_cmd, we have to modify _rx_cb, that impose complexity.

[haukepetersen]

now I see why I was so confused - I forgot that _at_cmd and _api_at_cmd were two different functions - long time since I wrote that code, sorry for all the nit-picking...

Just tested this PR -> there is still a bug: you need to call mutex_unlock(&(dev->tx_lock)); at the end of the _api_at_cmd function, otherwise it stays locked and hangs the second time this function is called...

[Yonezawa-T2]

tx_lock is already addressed in #4445.

[haukepetersen]

this seems not be be my day, overlooked that this depnends on #4445

[kYc0o]

@Yonezawa-T2 can you squash this too? code seems ok, I can test it ASAP, se let's Murdock check for errors.

[Yonezawa-T2]

Squashed.

[kYc0o]

will need rebase since #4445 has been merged...

[kYc0o]

ACK and try to merge it...

[miri64]

Needs backport.