ConnectivityFoundry / AwaLWM2M

Awa LWM2M is an implementation of the OMA Lightweight M2M protocol in C.
BSD 3-Clause "New" or "Revised" License
102 stars 66 forks source link

[WIP] Port Awalwm2m on RIOT #346

Closed ghost closed 7 years ago

ghost commented 7 years ago

RIOT compiles external libraries in pedantic mode. This pull request fixes all errors found when building Awa, excluding tests, in RIOT.

ghost commented 7 years ago

@boyvinall I do not compile C++ code in pedantic mode so I removed my changes concerning named initializers.

boyvinall commented 7 years ago

LGTM

@DavidAntliff @datachi7d any comments?

DavidAntliff commented 7 years ago

Looks like a lot of work - thanks for doing this. Most of it looks like missing casts but I've left some comments on a few lines - mostly just to try to understand the intent behind the changes. In particular, the changes to the logging macros look more complex and there's no explanation why. I understand it's probably to satisfy the compiler, but it would be good to know a bit more about the issue. Also, some actual struct names have leaked into the API headers - the original intent was to hide them behind uninstantiable struct declarations so that users cannot access their contents directly.

ghost commented 7 years ago

To give some context, all C code is compiled using pedantic mode. This is defined in boards/native/Makefile.include from RIOT, so I cannot change this.

@DavidAntliff Concerning this change about the logger, I added a new level of indirection to ensure that there are no empty variadic macros. Depending on the number of arguments, either LogStr_ or Log. I tried exporting -Wno-gnu-zero-variadic-macro-arguments but it does not have any effect because -pedantic is also defined. What I suggest is that this could be left as a patch in RIOT. This way, it will not get in this repository.

datachi7d commented 7 years ago

Hi @francois-berder-imgtec, these changes are good, but for building RIOT, we only need the static client and not the full API + daemons. Take a look at #348 I've used some of your changes to get a RIOT build going for the native platform. I didn't need to change the logging macros or hidden structs for the API. So perhaps we could revert logging and hidden struct changes but keep the rest?

DavidAntliff commented 7 years ago

@datachi7d good point - if we treat it like Contiki, then the daemon changes aren't really necessary. Most of the new casts look reasonable and are worth keeping, but as I mentioned in my pending review, some of them look a little strange (double casts).

ghost commented 7 years ago

@datachi7d Ok, I will compile only the client and update this pr. @DavidAntliff The double cast is quite hacky I reckon. I had to do this because you cannot cast a function pointer to a integeter pointer. But you can cast a function pointer to a number and then cast a number to a integer pointer...

boyvinall commented 7 years ago

Hi @francois-berder-imgtec, I've not managed to check through all your recent changes yet. But @datachi7d mentioned in #348 that use of CMake with RIOT was a bit awkward. So, I'm currently waiting to see something else there before merging anything CMake for RIOT.

datachi7d commented 7 years ago

@francois-berder-imgtec have you made any progress on using the POSIX sockets in RIOT? I found that the POSIX implementation was a bit lacking, making it difficult to use our linux network abstraction. These were the two hold ups:

  1. No support for non-blocking sockets or sockets that timeout, this can be seen in the recvfrom implementation in sys/posix/sockets/posix_sockets.c which blocks indefinitely :
    #ifdef MODULE_SOCK_UDP
        case SOCK_DGRAM:
            /* TODO: apply configured timeout */
            res = sock_udp_recv(&s->sock->udp, buffer, length, SOCK_NO_TIMEOUT,
                                &ep);
            break;
    #endif
  2. No support for DNS address resolution - there is a pull request to implement address resolution in the gnrc network stack, but it hasn't seen much progress in getting thru - RIOT-OS/RIOT#3823

The RIOT gnrc udp socket implementation would be the best bet for a more 'native' solution, but with the limitation of not being able to resolve hostnames. There is a uip (what contiki uses) implementation in pkg/emb6 which looks like it supports DNS resolution, so it might also be possible to reuse the contiki network abstraction instead.

ghost commented 7 years ago

@datachi7d Hello, I am currently debugging my application in RIOT. The bootstrap connection fails at one point: I know where the error starts happening and I suspect recvfrom is missing something. These are indeed 2 missing features. @boyvinall Not many changes in Awa. I am fixing RIOT and upstream my patches: https://github.com/RIOT-OS/RIOT/pull/6695 https://github.com/RIOT-OS/RIOT/pull/6697

ghost commented 7 years ago

@datachi7d I don't think I could use pkg/emb6 in awa as RIOT does not support build order dependency between packages. Also, I do not really understand why cmake is a bad idea on RIOT. Could you tell us a bit more about that ?

datachi7d commented 7 years ago

@francois-berder-imgtec - yes the bootstrap process will stall because the AwaStaticClient_Process function is blocked by recvfrom (AwaStaticClient_Process->coap_WaitMessage->coap_recieve->NetworkSocket_Read->readUDP->recvfrom) - that's where the requirement for non-blocking sockets comes from. Awa was designed to be single threaded, so the receive functions must not block in order for the AwaStaticClient_Process function to work correctly. The state machine for the bootstrapping process requires AwaStaticClient_Process to be called multiple times. If you change SOCK_NO_TIMEOUT in my comment above to a sensible value (~1 second) it will probably bootstrap.

Ignore that - I saw your changes in RIOT. They accepted a change to the timeout without questioning... hopefully nothing else is broken by that :)

datachi7d commented 7 years ago

CMake is a bad idea because RIOT does not setup a correct cross compile environment for CMake, take a look at pkg/relic/generate-cmake-xcompile.perl - it generates an override for the cmake variables. I found this had inconsistent behaviour across different hosts (ubuntu 14.04 and 16.04), and cross compilation targets. It would compile fine on one host for all targets but then produce errors on another host for specific targets. I had better luck using a makefile in RIOT which used the contiki makefiles to specify sources, here's an example using a modified version of your branch in RIOT:

# git clone https://github.com/datachi7d/RIOT.git --branch fix-RIOT --depth 1
# cd RIOT
# make -C examples/AwaLWM2M-client/

This compiles with near identical CFLAGS etc to that of RIOT, I had problems mimicking this behaviour with CMAKE reliably.

datachi7d commented 7 years ago

@francois-berder-imgtec good news! I had a quick look after merging in your changes to RIOT - the problem was that the recvfrom function was not populating the address so Awa couldn't figure out who was sending the bootstrap message. I've pushed the change to my RIOT branch which you can build from my example above.

It successfully bootstraps and registers, try it with the following:

Configure tap interface:

sudo ip tuntap add tap0 mode tap user ${USER}
sudo ip link set tap0 up
sudo ifconfig tap0 add 2001:1418:0100::1/64

Start bootstrap and server (separate terminals):

./build/daemon/src/bootstrap/awa_bootstrapd -e tap0 -f 6 --port 15683 -c config/contiki.bsc -v
./build/daemon/src/server/awa_serverd -e tap0 -f 6 -v

Run the example RIOT native app:

./examples/AwaLWM2M-client/bin/native/AwaLWM2M-client.elf tap0
boyvinall commented 7 years ago

Nice! Thanks @datachi7d

ghost commented 7 years ago

@datachi7d

I did manage to debug the posix interface in RIOT and found the bug causing the issue so I am now at the same level than you (bootstrap finishes and it registers to the server).

ghost commented 7 years ago

I pushed all the bug fixes I found to RIOT-OS/RIOT#6697.

ghost commented 7 years ago

I've created a new pr https://github.com/RIOT-OS/RIOT/pull/6771 to set timeout at posix layer.

cheekyhalf commented 7 years ago

retest AWS CI (erbium)

emmanuelsearch commented 7 years ago

@francois-berder @datachi7d @boyvinall is there an up-to-date branch somewhere with a RIOT pkg + example with the awa-lwm2m client, that I could try out? Just curious where we could pick up on our end ;)

boyvinall commented 7 years ago

Hi @emmanuelsearch. I'm a bit out of touch on this one at the moment, sorry. FYI, due to some internal restructuring at Imagination Technologies, we're likely to be finding a new home for Awa soon. I'm confident that we've got enough interested people to keep this running, but development will continue out of Imagination under a different github organisation.

@francois-berder can you share more info on the latest RIOT status? Thanks

emmanuelsearch commented 7 years ago

Hi @boyvinall thanks for the info. Count us in as interested people in awa. There'll be even more once we have a PR for a pkg in RIOT. As far as I understood it (e.g. this branch for instance ) we're probably close to that? Maybe I could try to contribute in the last steps towards that goal. Keep me (and more generally, us at RIOT) in the loop!

boyvinall commented 7 years ago

Great, I think we'd welcome your support in completing this. Many of us are busy scurrying around making plans at the moment, so may not be able to push this through just now. I'd suggest maybe waiting a day or two for @francois-berder or @datachi7d to respond, otherwise if you're able to pickup the pieces and contribute the pkg for RIOT then that would be great.

datachi7d commented 7 years ago

@emmanuelsearch I just had a look to see if my old RIOT branch (https://github.com/datachi7d/RIOT/tree/awa_static) would build the latest HEAD of Awa. There is a problem with missing macro - see #357. Otherwise it appears to build on the "native" board, which was bootstrapping correctly the last time I checked. I'll check with the latest version of Awa to see if it's still working.

In general, what really needs to be done is something in Awa's CI to ensure the RIOT components build correctly, since things like #357 can happen. What would be really nice is some basic tests in Awa to ensure the network abstraction for RIOT works correctly with Awa, something like checking that a "native" board with an Awa client bootstraps and connects to a server via a tap interface.

boyvinall commented 7 years ago

I meant to also pass on that I had some feedback from @francois-berder. He mentioned that his latest changes were in https://github.com/francois-berder/RIOT/tree/pkg-awa but I've not been able to check through those yet.