pkg/openthread: add FTD-MTD-NCP tests

[biboc]

This WIP PR implements OpenThread Full Thread Device (FTD), Minimal Thread Device (MTD) and Network Co-Processor (NCP) in RIOT.

FTD is a Thread device with routing-capability.
MTD does not have routing capability.
NCP is able to communicate with wpantund on a linux machine by UART.

[biboc]

It's a WIP PR because NCP does not work anymore and I've discovered a bug in our port of OpenThread.

The problem is still linked with otTasklets.
In OpenThread example, there is only one context with a while loop:

   while (1)
    {
        otTaskletsProcess(sInstance);
        PlatformProcessDrivers(sInstance);
    }

PlatformProcessDrivers(sInstance); checks if there is a radio packet, uart packet to send/receive and alarm that fired. So in contrary to RIOT, it pulls as fast as possible these three peripheral and at the same time execute pending task with otTaskletsProcess(sInstance);

By putting otTaskletsProcess(sInstance) in otTaskletsSignalPending() function, we might encounter an recusirve call because otTaskletsProcess() might call otTaskletsSignalPending() !!

So I don't really know where to add otTaskletsProcess(sInstance) since RIOT is event driven and does not have crazy while loop.

Is it clear?
@aabadie @jia200x

[jia200x]

@biboc Great to see this PR! I'm working in the sock part.

I see the problem. I just saw this. Maybe a msg_send_self in otTaskletsSignalPending could resolve this situation?

I will investigate further.

[jia200x]

@biboc could you try something like:

bool execute_taks=false;
void otTaskletsProcess(sInstance)
{
    execute_task = true;
}

...
msg_receive(&msg);
if(execute_task)
{
    otTaskletsProcess(sInstance);
    execute_task = false;
}
switch(...)
...

When otTasklestsProcess was in the beginning of the while loop, it was executed each time a msg arrived. I'm not sure if adding a flag resolves the issue, but I would give it a try.

Cheers

[biboc]

Something like this should do like POSIX example does:

while (1) {
        otTaskletsProcess(sInstance);
         if (!otTaskletsArePending(sInstance)) {
            msg_receive(&msg);
            switch (msg.type) {
                case OPENTHREAD_XTIMER_MSG_TYPE_EVENT:
                   ....
                case OPENTHREAD_JOB_MSG_TYPE_EVENT:
                    job = msg.content.ptr;
                    reply.content.value = ot_exec_command(sInstance, job->command, job->arg, job->answer);
                    msg_reply(&msg, &reply);
                    break;
            }
         }
    }

But still it does not work as expected

[biboc]

My last edits work great! This is the right way to implement otTask.
Now, NCP is not working as expected. Wpantund detects the NCP, it can send commands but "scan" and "form" commands fail. Can you have a try @jia200x?

[biboc]

Works better now!
Scan is working, form a network is still broken

[jia200x]

Hi @biboc.

For some reason I'm have 2 leader nodes with the openthread_ftd example. I'm investigating what might cause that.

I will do some NCP tests as well.

[jia200x]

@biboc the scan is working. But, as said before, there's something wrong with the core. That could be related to the "form" problem.

Cheers

[jia200x]

@biboc after some attempts, 2 ftd nodes are working ok. I will test it on Iot-LAB

Cheers

[jia200x]

@biboc just discovered something strange:

There are some commands I'm unable to execute with the OpenThread CLI.
E.g:

extpanid DEBA7AB1E5EAF00D
Error 14: InvalidState

Have you had this behavior before?

[biboc]

To change the panid you need to "thread stop" before

[biboc]

Your test is great help, thanks

[biboc]

I upgraded OpenThread to the last version.
@jia200x, can you have a look at platform_radio.c: send_pkt(): otPlatRadioTxDone(aInstance, &sTransmitFrame, NULL, OT_ERROR_NONE);
I use NULL since I can't get the ACK frame from netdev (even with at86rf2xx since it is automated)
What do you think?

[jia200x]

hi @biboc

If I'm not wrong, the NETDEV_EVENT_TX_COMPLETE should be triggered only if there's an ACK (@miri64 @OlegHahm ??). Otherwise, the NETDEV2_EVENT_NO_ACK will be triggered.
What's the behavior you are having with ACK?

[biboc]

The problem here is the change in OpenThread API, instead of using otPlatRadioTransmitDone, we should use otPlatRadioTxDone:

/**
 * The radio driver calls this method to notify OpenThread that the transmission has completed,
 * this callback pass up the ACK frame, new add platforms should use this callback function.
 *
 * @param[in]  aInstance  The OpenThread instance structure.
 * @param[in]  aFrame     A pointer to the frame that was transmitted.
 * @param[in]  aAckFrame  A pointer to the ACK frame, NULL if no ACK was received.
 * @param[in]  aError     OT_ERROR_NONE when the frame was transmitted, OT_ERROR_NO_ACK when the frame was
 *                        transmitted but no ACK was received, OT_ERROR_CHANNEL_ACCESS_FAILURE when the transmission
 *                        could not take place due to activity on the channel, OT_ERROR_ABORT when transmission was
 *                        aborted for other reasons.
 *
 */
extern void otPlatRadioTxDone(otInstance *aInstance, otRadioFrame *aFrame, otRadioFrame *aAckFrame,
                              otError aError);

/**
 * The radio driver calls this method to notify OpenThread that the transmission has completed,
 * this function is going to be deprecated, new add platfroms should not use this callback function.
 *
 * @param[in]  aInstance      The OpenThread instance structure.
 * @param[in]  aFrame         A pointer to the frame that was transmitted.
 * @param[in]  aFramePending  TRUE if an ACK frame was received and the Frame Pending bit was set.
 * @param[in]  aError         OT_ERROR_NONE when the frame was transmitted, OT_ERROR_NO_ACK when the frame was
 *                            transmitted but no ACK was received, OT_ERROR_CHANNEL_ACCESS_FAILURE when the transmission
 *                            could not take place due to activity on the channel, OT_ERROR_ABORT when transmission was
 *                            aborted for other reasons.
 *
 */
extern void otPlatRadioTransmitDone(otInstance *aInstance, otRadioFrame *aFrame, bool aFramePending,
                                    otError aError);

So I need to pass the ACK frame as mentioned here:
* @param[in] aAckFrame A pointer to the ACK frame, NULL if no ACK was received.
But netdev don't provide it.

Is it clearer? @jia200x

[biboc]

We've fixed UART problem for NCP with buffering the current frame before giving it to OpenThread, it works great!

Now I would like @jia200x to have a look at the otPlatRadioTxDone in my previous comment so we can end this PR

[jia200x]

hi @biboc.
Sorry for the delay, I was busy with work.

Your buffering solution is very smart! Congratz.

Concerning the ACK part, IMHO I would enable OPENTHREAD_CONFIG_LEGACY_TRANSMIT_DONE and use the otPlatRadioTransmitDone, until we find another solution. There's no way to retrieve the ACK from the driver with Auto ACK. Also, passing Ack=NULL may have side effects.

Cheers!

[biboc]

Yes they use it only for ackFrame->GetPower() finally but you're right let's use old function as long as we can

[jnohlgard]

So it's not the ACK itself, but the LQI or RSSI of the ACK that you need?

[biboc]

@gebart, it might be only for this yes but since we can still use old function I'll vote to use old function and not "hack" the function by filling only receiving power and even this I'm not sure power is updated when an ACK is received.

[Hyungsin]

@kaspar030, small correction for the title: pkt -> pkg.

[kaspar030]

@kaspar030, small correction for the title: pkt -> pkg.

thx!

[Hyungsin]

@jia200x @biboc @aabadie, I opened another pull request which solves a bit alignment error in the openthread uart driver. I put some comments. Please merge it before testing this PR.
biboc#2

[jia200x]

@aabadie @Hyungsin I have a demo tomorrow so I couldn't find time to work on this. I'm back on Monday.

[jia200x]

This EDBG issue is still happening after the fix:

wpantund[12457]: State change: "offline" -> "associating"
wpantund[12457]: State change: "associating" -> "associated"
wpantund[12457]: NCP => Framing error 6: [80 03 72 68 00 60 0D 22 DD 00 40 3A 40 FD DE AD 00 BE EF 00 00 0D 7E DF 81 95 62 BD 76 FD DE AD 00 BE EF 00 00 37 E0 42 FB]
wpantund[12457]: NCP => Framing error 6: [80 03 72 68 00 60 0D 22 DD 00 40 3A 40 FD DE AD 00 BE EF 00 00 0D 7E DF 81 95 62 BD 76 FD DE AD 00 BE EF 00 00 37 E0 42 FB]
wpantund[12457]: NCP => Framing error 6: [80 03 72 68 00 60 0D 22 DD 00 40 3A 40 FD DE AD 00 BE EF 00 00 0D 7E DF 81 95 62 BD 76 FD DE AD 00 BE EF 00 00 37 E0 42 FB]
wpantund[12457]: NCP => Framing error 6: [80 03 72 68 00 60 0D 22 DD 00 40 3A 40 FD DE AD 00 BE EF 00 00 0D 7E DF 81 95 62 BD 76 FD DE AD 00 BE EF 00 00 37 E0 42 FB]
wpantund[12457]: NCP => Framing error 6: [80 03 72 68 00 60 0D 22 DD 00 40 3A 40 FD DE AD 00 BE EF 00 00 0D 7E DF 81 95 62 BD 76 FD DE AD 00 BE EF 00 00 37 E0 42 FB]
wpantund[12457]: NCP => Framing error 6: [80 03 72 68 00 60 0D 22 DD 00 40 3A 40 FD DE AD 00 BE EF 00 00 0D 7E DF 81 95 62 BD 76 FD DE AD 00 BE EF 00 00 37 E0 42 FB]
wpantund[12457]: NCP => Framing error 6: [80 03 72 68 00 60 0D 22 DD 00 40 3A 40 FD DE AD 00 BE EF 00 00 0D 7E DF 81 95 62 BD 76 FD DE AD 00 BE EF 00 00 37 E0 42 FB]
wpantund[12457]: NCP => Framing error 6: [80 03 72 68 00 60 0D 22 DD 00 40 3A 40 FD DE AD 00 BE EF 00 00 0D 7E DF 81 95 62 BD 76 FD DE AD 00 BE EF 00 00 37 E0 42 FB]
wpantund[12457]: NCP => Framing error 6: [80 03 72 68 00 60 0D 22 DD 00 40 3A 40 FD DE AD 00 BE EF 00 00 0D 7E DF 81 95 62 BD 76 FD DE AD 00 BE EF 00 00 37 E0 42 FB]
wpantund[12457]: NCP => Framing error 6: [80 03 72 68 00 60 0D 22 DD 00 40 3A 40 FD DE AD 00 BE EF 00 00 0D 7E DF 81 95 62 BD 76 FD DE AD 00 BE EF 00 00 37 E0 42 FB]
wpantund[12457]: NCP => Framing error 6: [80 03 72 68 00 60 0D 22 DD 00 40 3A 40 FD DE AD 00 BE EF 00 00 0D 7E DF 81 95 62 BD 76 FD DE AD 00 BE EF 00 00 37 E0 42 FB]
wpantund[12457]: NCP => Framing error 6: [80 03 72 68 00 60 0D 22 DD 00 40 3A 40 FD DE AD 00 BE EF 00 00 0D 7E DF 81 95 62 BD 76 FD DE AD 00 BE EF 00 00 37 E0 42 FB]
wpantund[12457]: NCP => Framing error 6: [80 03 72 68 00 60 0D 22 DD 00 40 3A 40 FD DE AD 00 BE EF 00 00 0D 7E DF 81 95 62 BD 76 FD DE AD 00 BE EF 00 00 37 E0 42 FB]
wpantund[12457]: NCP => Framing error 6: [80 03 72 68 00 60 0D 22 DD 00 40 3A 40 FD DE AD 00 BE EF 00 00 0D 7E DF 81 95 62 BD 76 FD DE AD 00 BE EF 00 00 37 E0 42 FB]
wpantund[12457]: NCP => Framing error 6: [80 03 72 68 00 60 0D 22 DD 00 40 3A 40 FD DE AD 00 BE EF 00 00 0D 7E DF 81 95 62 BD 76 FD DE AD 00 BE EF 00 00 37 E0 42 FB]
wpantund[12457]: NCP => Framing error 6: [80 03 72 68 00 60 0D 22 DD 00 40 3A 40 FD DE AD 00 BE EF 00 00 0D 7E DF 81 95 62 BD 76 FD DE AD 00 BE EF 00 00 37 E0 42 FB]
wpantund[12457]: NCP => Framing error 6: [80 03 72 68 00 60 0D 22 DD 00 40 3A 40 FD DE AD 00 BE EF 00 00 0D 7E DF 81 95 62 BD 76 FD DE AD 00 BE EF 00 00 37 E0 42 FB]
wpantund[12457]: NCP => Framing error 6: [80 03 72 68 00 60 0D 22 DD 00 40 3A 40 FD DE AD 00 BE EF 00 00 0D 7E DF 81 95 62 BD 76 FD DE AD 00 BE EF 00 00 37 E0 42 FB]
wpantund[12457]: NCP => Framing error 6: [80 03 72 68 00 60 0D 22 DD 00 40 3A 40 FD DE AD 00 BE EF 00 00 0D 7E DF 81 95 62 BD 76 FD DE AD 00 BE EF 00 00 37 E0 42 FB]
wpantund[12457]: NCP => Framing error 6: [80 03 72 68 00 60 0D 22 DD 00 40 3A 40 FD DE AD 00 BE EF 00 00 0D 7E DF 81 95 62 BD 76 FD DE AD 00 BE EF 00 00 37 E0 42 FB]
wpantund[12457]: NCP => Framing error 6: [80 03 72 68 00 60 0D 22 DD 00 40 3A 40 FD DE AD 00 BE EF 00 00 0D 7E DF 81 95 62 BD 76 FD DE AD 00 BE EF 00 00 37 E0 42 FB]

That's whatI get when I ping. If I modify the packet size, it works

[jia200x]

I'm pretty sure is a Flow Control issue, as seen here. In that case, the fix is not related to this PR.

[jia200x]

@Hyungsin @aabadie

Are you OK in splitting this PR in FTD/MTD support and NCP?

• The FTD nodes are working ok and the blocking part is NCP, where there are chances the issue is not RIOT related
• The NCP is just one of the several configurations as seen in the official openthread site. It's not the only way to provide external IPv6 connectivity (e.g RIOT nodes should be compatible with standard Thread Border Routers, not necessarily related to OpenThread).
• It's possible to use one of the OpenThread platforms as an NCP directly from the OpenThread repository. For instance the samr21-xpro is supported here. The only benefit of having a RIOT NCP is the fact that it's possible to use other boards. But since there's only support for at86rf2xx now, I don't think it would make a huge difference to postpone a little bit the NCP part.

Also, merging the FTD part can speed up the adoption to other radios, and improve the OpenThread port.

Comments?

[jia200x]

BTW splitting this PR should be fast. All NCP parts are marked via #ifdef.
If you agree with the idea, I would use the OpenThread release (last commit Dec 2017).

[Hyungsin]

I agree :) Let’s split and merge this!

[Hyungsin]

@jia200x, just in case, This is what you mean right? https://github.com/openthread/openthread/releases/tag/thread-reference-20170716

[aabadie]

This is what you mean right?

@Hyungsin, exactly! The tag name is confusing but the version is from last November. Is this version ok for you ?

[Hyungsin]

@jia200x, sure.
The version is used in the current code? or do you need to update further?

[jia200x]

@Hyungsin @aabadie it's quite confusing, since I checked this is the last commit of OpenThread (July 2017). However, I still tend to think it's good to keep the Thread Reference implementation (it will be easier for certification, etc).

The current code doesn't run with that version (function prototypes changed) but it's not a big deal to make it work again. There are some features that are not present in the old version (specially CLI) but most important features are there (MTD, SED, Border Router support, etc). @Hyungsin do you have out of your head any features we might miss with the release?

In any case, I'm pretty sure there will come another Release and we will need this PR.

[Hyungsin]

@jia200x,

I think we don't have to worry about features at this time, which can be added in future PRs.
I see this PR as the basic stepping stone, which compiles well when using the 20170716 release.

So if you can work on re-prototyping, please go ahead and let it merged :)
The function prototypes have been changed after the release anyway.

After this PR, may be you can keep working on NCP and I can reflect the features I have added so far, including re-re-prototyping.

Given that the current NCP works with small packets, can we also merge the current NCP part with the comments about the limitation?

[Hyungsin]

Any update here?

[jia200x]

@Hyungsin just opened #9336 . It should be fast to merge since this PR had some reviews.

[Hyungsin]

@jia200x. Cool!! Thanks!

[miri64]

#9336 was merged. So I guess this can be closed.

[jia200x]

it still has some code to be ported (NCP, etc), but closing it won't hurt anyone :)

[miri64]

Then I archive it.