[WIP] examples: add Wakaama/LwM2M example client#8610
[WIP] examples: add Wakaama/LwM2M example client#8610moenoel wants to merge 5 commits intoRIOT-OS:masterfrom
Conversation
|
This can be of interest: PR #7615 |
rfuentess
left a comment
There was a problem hiding this comment.
My review is focused mainly over the DTLS implementation.
Once I got time, I'll be testing this client with a demo server.
examples/wakaama/dtlsconnection.c
Outdated
|
|
||
| dtls_context_t *get_dtls_context(dtls_connection_t *connList) { | ||
| if (dtlsContext == NULL) { | ||
| dtls_init(); |
There was a problem hiding this comment.
This should be moved to main.c
dtls_init() must be called only once time by reboot (This is not explicit yet in master:examples/dtls-echo).
Otherwise, there is risk of unexpected behavior when a second or posterior context is created
examples/wakaama/dtlsconnection.c
Outdated
| connP->next = connList; | ||
| connP->noSec = false; | ||
|
|
||
| connP->dtlsSession = (session_t *)malloc(sizeof(session_t)); |
There was a problem hiding this comment.
Eventually you need to remove the malloc/free calls. Maybe PR #7651 can be used here.
There was a problem hiding this comment.
The Wakaama package itself uses malloc/free as well, so it would probably be a relatively big undertaking to move all that to memarray. Not sure that falls under the scope of this pull request, especially since memarray isn't merged yet.
There was a problem hiding this comment.
Sadly, if you stay with malloc/free, there will be a lot of problems with many boards. Because is not that well supported on many RTOS (including RIOT). You can use malloc() the first time, but free() is not going to work.
Also, if you are not sure of having enough memory, you will have a nightmare catching the overflows.
There was a problem hiding this comment.
Alright, that's a very good argument to do this sooner rather than later. I just rebased the changes from #7651 into my branch and started moving things over. I'll have to see what to do about variable sized buffer allocations and such.
I'll also have to look into how to best apply this to the Wakaama package.
There was a problem hiding this comment.
I just replaced all the malloc/free stuff with memarray. Though I'm not sure how hacky/evil the thing is I did for lwm2m_malloc() and lwm2m_free() in platform.c for variable sized allocations.
examples/wakaama/dtlsconnection.c
Outdated
| } | ||
|
|
||
| // reset current session | ||
| dtls_peer_t *peer = dtls_get_peer(connP->dtlsContext, connP->dtlsSession); |
There was a problem hiding this comment.
It's not less aggressive in resources to use dtls_renegotiate() ?
Or maybe not, as you truly want to terminate the relation between the peers.
There was a problem hiding this comment.
This is code I've taken as is from the Wakaama example, so I'm not entirely sure which of the two should be used here.
There was a problem hiding this comment.
Ok. For purposes of comparing performance it will be better not touching it then.
Unless that another reviewer has a different opinion.
examples/wakaama/dtlsconnection.c
Outdated
|
|
||
| static dtls_context_t *dtlsContext; | ||
|
|
||
| static int gnrc_sending(char *addr_str, char *data, size_t data_len, |
There was a problem hiding this comment.
This is working, but is legacy and is one of the main things I want to remove with PR #7615
There was a problem hiding this comment.
Will GNRC be deprecated or is it ok if it stays like this for now? I'm planning to port the Wakaama server as well in the future, so I could use gnrc_sock_udp for that and move the client over as further down the line.
There was a problem hiding this comment.
GNRC per se will not be deprecated, so far as I know. However, my implementation of that function is not very optimal (just look to that timer :( ).
The part of UDP_sock in the PR #7615 remove that issue (I still need a delay for the client, but can be as low as 100 uS). So, I strongly suggest to upgrade this part while your PR is not merged.
There was a problem hiding this comment.
I see your point. I'll try to get to that next week.
There was a problem hiding this comment.
I've moved to socks now and the thing compiles and runs on native, but it doesn't seem to use the tap interface anymore, so I can't really test it. Is that expected behavior, or did I make a mistake somewhere?
There was a problem hiding this comment.
After banging my head against a wall for several days, it turns out that the socks API doesn't like sending packets when there's no netif ID set on the remote endpoint and the target IPv6 address is link-local. sock_udp_send() does not treat that as an error though, and in fact queues a packet that will (permanently ?) clog up the send queue.
I built in a workaround for that now.
examples/wakaama/dtlsconnection.c
Outdated
| .write = send_to_peer, | ||
| .read = read_from_peer, | ||
| .event = NULL, | ||
| //#ifdef DTLS_PSK |
There was a problem hiding this comment.
You should be OK enabling those lines. the DTLS_ECC and DTLS_PSK flags are handled in the Makefile.
examples/wakaama/Makefile
Outdated
| CFLAGS += -DDTLS_PSK | ||
|
|
||
| # NOTE: This adds support for TLS_ECDHE_ECDSA_WITH_AES_128_CCM_8 | ||
| CFLAGS += -DDTLS_ECC |
There was a problem hiding this comment.
After commit bb8ca70 and because you were testing only the PSK mode, it's a good idea to disable this line (or remove it).
You can use git commit --fixup==bb8ca70d4d33212d3a7b37b57fc1c25a2f38160f for this change. Though is not relevant because both commits will be squashed.
There was a problem hiding this comment.
Since, as you said, all that will be squashed anyway, I just snuck that into the latest commit, since it's not that dramatic of a change.
rfuentess
left a comment
There was a problem hiding this comment.
I was checking this before the rebase with memarray. But, I think these observations remain true even with the integration with sock.
examples/wakaama/lwm2mconfig.h
Outdated
| * The host part of the URI MUST be a valid IPv6 address. Host names | ||
| * can not be resolved at this time. | ||
| */ | ||
| #define LWM2M_SERVER_URI "coaps://[fe80::5838:f4ff:fed6:6805]" |
There was a problem hiding this comment.
Probably this should be inside of #ifndef clause. Thus, we can define the server at fly.
There was a problem hiding this comment.
The idea was to edit the header before building (as I've described in the Readme file). If you think it's better to have these things in the Makefile, I might as well get rid of lwm2mconfig.h.
There was a problem hiding this comment.
My comment with this specific issue is not absolute but a reference. IMHO this approach make the things more easy for testing.
lwm2mconfig.h could have default values. By example it could be coaps://[fd00:dead:beef::1] due that is the default value when dist/tools/ethos/start_network.sh is called.
examples/wakaama/lwm2mconfig.h
Outdated
| * PSK KEY of LWM2M_SERVER_URI in hexadecimal. | ||
| * Set to NULL to not encrypt traffic. | ||
| */ | ||
| #define LWM2M_SERVER_PSK_KEY "39fe6611deb7713c6069" |
There was a problem hiding this comment.
Is this the correct PSK key used by the server given in the README?
My attempts to make the DTLS handhsake fails as soon the client key exchange record is sent.
There was a problem hiding this comment.
As I said, I'm using Eclipse Leshan for testing. You have to set your own keys for each client there in the web interface. This is a key I've generated myself.
There was a problem hiding this comment.
ah, my bad. I should had read again the README.md before testing the server.
examples/wakaama/lwm2mclient.c
Outdated
| DEBUG("connection_handle_packet(%d)\r\n", pckt_rcvd_size); | ||
| int result = connection_handle_packet(connP, packet_rcvd, pckt_rcvd_size); | ||
| if (0 != result) { | ||
| printf("error handling message %d\n", result); |
There was a problem hiding this comment.
It's seems that you are not handling correctly the scenario where the DTLS handshake fails because the native instance is abruptly ended.
There was a problem hiding this comment.
I added some code to handle timeouts during DTLS handshakes.
examples/wakaama/lwm2mclient.c
Outdated
| free_object_device(objArray[2]); | ||
| acl_ctrl_free_object(objArray[3]); | ||
|
|
||
| return NULL; |
There was a problem hiding this comment.
Resources are not being released correctly, I'm forced to kill the instance native to be able to quit.
There was a problem hiding this comment.
Oh, so that's why it doesn't terminate with ctrl+c on native? I was wondering why that is.
I've replaced the memarray stuff in platform.c with malloc/free again and looked what valgrind says. I did a small fix, but there seems to be a leak from TinyDTLS. Not sure if that's it or what to do about it.
There was a problem hiding this comment.
It was not memarray. This issue is present in the bb8ca70 commit.
There was a problem hiding this comment.
I know, it's been there from the start. I only disabled memarray so valgrind would also detect unfreed memory in the memarray stuff (not that it would matter, since all that is statically allocated with memarray, now that I think about it).
Anyway, the client-thread seems to terminate. At least ps doesn't show it anymore after a ctrl+c. I seem to have forgotten to close the socks allocated for the individual connection, but since I still can't get a network connection under native, none ever gets created anyway, aside from the listener.
Not sure how to debug this further/what else to look at.
There was a problem hiding this comment.
I still have no idea which blocking resources are not cleaned up properly. For the time being, I added a workaround so the programm can be closed on native without having to kill the process (exit command in the shell).
I'll try to review the changes with sock tomorrow to identify what could had happened. |
examples/wakaama/dtlsconnection.c
Outdated
| } | ||
| if (gnrc_sending(addr_str, (char *)buffer, length, | ||
| connP->dtlsSession->port) != 1) { | ||
| length = sock_udp_send(&(connP->sock), buffer, length, NULL); |
There was a problem hiding this comment.
But I do. In connection_create() I set the remote sock_udp_ep_t and pass it to connection_new_incoming() which makes a SOCK_IPV6_EP_ANY local endpoint and passes both to sock_udp_create().
The only sock that doesn't have a remote endpoint set is the listener in lwm2mclient.c but that never gets passed to a sock_udp_send() call, unless I'm missing something.
examples/wakaama/dtlsconnection.c
Outdated
| sock_udp_ep_t remote = { .family = AF_INET6 }; | ||
| char *defaultport; | ||
| sock_udp_ep_t remote = { | ||
| .family = AF_INET6, |
There was a problem hiding this comment.
Minor comment, you can use
sock_udp_ep_t remote = SOCK_IPV6_EP_ANY
See here
Ah yes, this behavior is recently, there is a "fix" available in PR #8470. Sorry, I should had provided this information much more sooner. You can see an alternative to your solution in the gcoap example. |
|
Tested with DTLS. Is working now. But partially. I'm getting this: And the leshan server (without boostrapping) output is this: In the current state of this PR, the behavior of your client and the demo server are the expected? |
|
Also, please update your OP for the following points: |
|
What seems to be only partially working for you? I've never seen the Leshan messages about an "illegal reflective access" on my end, but otherwise this looks fine to me (granted, the debug output is kind of messy right now; I gotta clean that up at some point). The client reports to be in "STATE_READY", which means the DTLS handshake succeeded and it successfully registered with the Leshan server. You should now be able to interact with the client via the Leshan web interface running on |
rfuentess
left a comment
There was a problem hiding this comment.
Finally, I got time to review this PR a little more.
The elements of DTLS seems to be fine now.
But we are having hardfaults when tested with other boards than native. And I suspected that is related to the current problem with native .
examples/wakaama/Makefile
Outdated
| # Add a routing protocol | ||
| USEMODULE += gnrc_rpl | ||
| # This application dumps received packets to STDIO using the pktdump module | ||
| USEMODULE += gnrc_pktdump |
There was a problem hiding this comment.
You can remove this module in a safe way.
|
|
||
| Build, flash and start the application: | ||
| ``` | ||
| export BOARD=your_board |
There was a problem hiding this comment.
(Very minor comment) You can modify this to the following:
BOARD=<your_board> make all flash term
Though, the rule flash call all yet, I find it more clear to add all plus the line 22 will be still valid.
| ======================= | ||
|
|
||
| At this time, only the mandatory objects for a client are present. The | ||
| *server*, *security* and *access control* objects are taken as is from |
There was a problem hiding this comment.
Please, clarify something for me.
This statement made me understand that CoAPS is mandatory, and CoAP will not be compiled/supported. However, the waakama repository marks CoAPS compilation as optional. Also, the way the server is working, it's enabling CoAP and CoAPS (two ports). Also, even the OP of this PRs give the idea that can be tested with or without DTLS.
So, what is expected behavior for your port to RIOT? Only CoAPS or also CoAP ?
There was a problem hiding this comment.
CoAP can be used by not specifying any keys (in this case, when LWM2M_SERVER_PSK_ID and LWM2M_SERVER_PSK_KEY are set to NULL) as that will bypass calling DTLS functions (see e.g. connection_handle_packet()). The wakaama example makes it possible to not compile with TinyDTLS so only CoAP will work, but with TinyDTLS, both will. Because of that, I did not make the effort to port the non-DTLS version.
There was a problem hiding this comment.
Ah, now is more clear for me.
IMHO there is also a factor to consider for RIOT: If DTLS is not going to be used, it should not be compiled in the first place.
This could help us to reduce the complexity of the testings by removing tinyDTLS stack temporally. Particularly, if all the lines with DTLS are inside of the files dtlsconnection.* . Otherwise, could be a future work to consider.
| the credentials set in `lwm2mconfig.h` via the web interface. Since the | ||
| example bootstrap server doesn't seem to support setting DTLS credentials | ||
| for the server itself via the web interface, you'll have to create a | ||
| new client config, edit `data/bootstrap.json` and set `securityMode`, |
There was a problem hiding this comment.
Probably a minimum example for data/bootstrap.json should be provided in your README.md.
This would make the things more easy for other users that are not well versed into the LW2M.
There was a problem hiding this comment.
I just did, along with an explanation of how to read the file in the readme file.
examples/wakaama/README.md
Outdated
| Example output | ||
| ============== | ||
|
|
||
| When registering directly to an LwM2M server, with DTLS, without boostrapping: |
There was a problem hiding this comment.
If I understand correctly, this specific output will be only possible when LWM2M_WITH_LOGS is enabled. If this the case, this statement should make a notion about it.
There was a problem hiding this comment.
You're right. I think I had that set by default, so that the code compiled as is would match the example output, but I might have commented it out while testing things and forgotten to put it back in before committing a change.
I'll specify which debug settings to use to get a matching debug output in README.md.
examples/wakaama/dtlsconnection.h
Outdated
| extern "C" { | ||
| #endif | ||
|
|
||
| #define LWM2M_STANDARD_PORT_STR "5683" |
There was a problem hiding this comment.
The definition of the ports should not be at lwm2mconfig.h ?
There was a problem hiding this comment.
You're right. These seem to be remnants from the code to parse cli arguments of the original. I even made duplicate defines in dtlsconnection.c for some reason. I just cleaned all that up and moved it to lwm2mconfig.h.
|
|
||
| #include "lwm2mconfig.h" | ||
| #include "memarray_limits.h" | ||
|
|
There was a problem hiding this comment.
Because you have DEBUG lines. Please add the following:
#define ENABLE_DEBUG (0)
#include "debug.h"
There was a problem hiding this comment.
This is already defined in lwm2mconfig.h.
| #include <string.h> | ||
| #include <sys/types.h> | ||
| #include <unistd.h> | ||
|
|
There was a problem hiding this comment.
Because you are adding DEBUG () lines please add the following:
#define ENABLE_DEBUG (0)
#include "debug.h"
This is required for all the C files that make use of DEBUG().
There was a problem hiding this comment.
Same as above. This is already defined in lwm2mconfig.h.
There was a problem hiding this comment.
I see that's already defined there, but the style of RIOT is to enable debug only for the C file we want to debug. If lwm2mconfig.h is used somewhere else the code will be activated for debug without any reason. Thus, we prefer to explicitly add the debug part here.
There was a problem hiding this comment.
This is an style issue so don't care about it for now. First we need to validate that your approach is good and your implementation works properly. We can discuss this afterwards.
examples/wakaama/lwm2mclient.c
Outdated
| } | ||
|
|
||
| /* | ||
| * Finally when the loop is left smoothly - asked by user in the command line |
There was a problem hiding this comment.
I still need to read your code a little more (and the package).
But, are we sure we want to terminate this thread? Is this one who has all DTLS context and even Wakama data.
Also, you are not adding any command to the shell as suggested by this line.
There was a problem hiding this comment.
The exit command is added in main.c.
And as the main loop can only be cleanly termianted by changing the value of g_quit, which currently only happens in the SIGINT signal handler. This was basically just meant to make the program quitable on native (which still doesn't work properly yet, hence the exit shell command).
| mkdir -p "$(PKG_BUILDDIR)/riotbuild" | ||
| cp $(PKG_BUILDDIR)/core/*.c $(PKG_BUILDDIR)/core/*.h $(PKG_BUILDDIR)/riotbuild | ||
| cp $(PKG_BUILDDIR)/core/er-coap-13/*.c $(PKG_BUILDDIR)/core/er-coap-13/*.h $(PKG_BUILDDIR)/riotbuild | ||
| cp $(PKG_BUILDDIR)/examples/client/object_server.c $(PKG_BUILDDIR)/riotbuild |
There was a problem hiding this comment.
Sadly this approach is not valid.
The package can not be modified for each application or example.
There was a problem hiding this comment.
The object implementations taken from the example client should not need to be modified for each application, as they implement a standardized data model but nothing platform or application specific. They each expose functions to create/destroy object instances with customized data.
That is also why I did not take object_device.c, as that actually is an object that needs to be modified for each device/application.
Check this output from your README.md: Also, I think that the commit c89f4b8 is a clear signal that the PR is not ready.
So far I had been sending the reboot request to the node. However, once the "STATE_READY" is reached, the node eventually crashes. |
I have gotten to test the code on an stm32f4discovery myself by now and also experienced hardfaults. At first I got an ISR stack overflow and after working around that by hardcoding a bigger stack, I got a hardfault for trying to access misaligned memory. I haven't had the chance to take a deeper look at that yet, though. |
Huh. I actually never let the client run long enough to notice this. The crash seems to be due to a segfault in the network stack. At least that's what gdb says on native: |
9f791d4 to
65d19b9
Compare
|
Looks like the crash was actually caused by an (old) bug in the network stack. Updating master and rebasing on that seems to have fixed it. |
|
The hardfaults I got on stm32f4discovery are now also fixed (for me at least) by adding |
|
Tested with edfb21f (after rebase). Some observations (for native only):
Boards samr21-xpro and iotlab-m3 are crashing due overflow, and sadly that is all the hardware available to me. |
I can't reproduce that on my end. I tried multiple times, sometimes recompiling between tests and also with multiple reboots of the RIOT system.
I still have no clue what causes this. The wakaama thread itself terminates and vanishes from the process list (going by
Did you also change the protocol scheme from
I'll see what I can do about making TinyDTLS optional to reduce the resource footprint. Maybe that'll also give an insight on why the process won't terminate on native. |
Sorry, my fault, I forgot to update that line. I have now running at the same time both types of clients (CoAP and CoAPS). |
True, it's a solid point. In an offline talk with @aabadie, he proposed a similar idea to yours that could be useful (please, @aabadie correct me if I'm wrong). this idea consists of using the following structure for All the objects implementations (including those in the original wakaama examples) can be there. And then copied into the RIOT build directory. The patch: git-download
mkdir -p "$(PKG_BUILDDIR)/riotbuild"
cp $(PKG_BUILDDIR)/core/*.c $(PKG_BUILDDIR)/core/*.h $(PKG_BUILDDIR)/riotbuild
cp $(PKG_BUILDDIR)/core/er-coap-13/*.c $(PKG_BUILDDIR)/core/er-coap-13/*.h $(PKG_BUILDDIR)/riotbuild
cp $(RIOTPKG)/wakaama/static/*.c $(PKG_BUILDDIR)/riotbuildFor the header files, I would leave them on the application directory. And for the |
An alternative would make this modification to # Specific the server URI address (NOTE: Domain names not supported yet)
SERVER_URI?='"coaps://[fd00:dead:beef::1]"'
ifneq ( , $(SERVER_URI))
CFLAGS += -DLWM2M_SERVER_URI=$(SERVER_URI)
endif
ifneq (,$(findstring coaps,$(SERVER_URI)))
$(info Enabling tinyDTLS)
# NOTE: Add the package for TinyDTLS
USEPKG += tinydtls
# NOTE: Without ECC, this defaults to 100 for RIOT which is not enough.
CFLAGS += -DDTLS_MAX_BUF=200
#Any other configuration required for tinyDTLS
else
# Any special variable required to take TinyDTLS place.
endifAnd the following changes to ...
#ifndef LWM2M_SERVER_URI
#define LWM2M_SERVER_URI "coap://[fd00:dead:beef::1]"
#endif
..
/*
* PSK ID of LWM2M_SERVER_URI.
*/
#ifdef MODULE_TINYDTLS
#define LWM2M_SERVER_PSK_ID "fooTestRIOT"
// #define LWM2M_SERVER_PSK_ID NULL
/*
* PSK KEY of LWM2M_SERVER_URI in hexadecimal.
* Set to NULL to not encrypt traffic.
*/
#define LWM2M_SERVER_PSK_KEY "39fe6611deb7713c6069"
#else
#define LWM2M_SERVER_PSK_ID NULL
#define LWM2M_SERVER_PSK_KEY NULL
#endifWith this you can switch with a single line between CoAP and CoAPS and the next step would be excluding the source files used for DTLS and adding NOTE: Once PR #7615 is merged, your Makefile would only require the two lines stated in my suggestion. EditI did a quick fix on the suggestion for the Makefile, to make possible the suppression of the line |
|
I was testing a little more with the m3-boards (using only CoAP):
For CoAPS, I I bumped tinyDTLS to the version with memarray support. This makes the native more stable when releasing tinyDTLS resources. But, it doesn't seem to have an impact in the other crash. |
examples/wakaama/lwm2mclient.c
Outdated
| int addressFamily; | ||
| } client_data_t; | ||
|
|
||
| void handle_sigint(int signum) { |
There was a problem hiding this comment.
Since the files are already part of the Wakaama codebase and bumping the Wakaama version might involve fixing the patches in I also now made TintyDTLS optional via pre-processor directives, so I don't think there's anything that would go in the dtls sub-directory.
Do you know which of the DEBUG lines causes the first problem? I don't have that board and I can't reproduce either of these problems on native or with my stm32f4discovery. |
|
Finally I got more time for this PR. Also, I recovered my m3 nodes so, I'm able to test this on physically nodes. First, its seems that Also, the issue with the overflow is happening due to the definitions used in internal.h. Once I disabled Wakaama debug log ( This is the PS output when tinyDTLS support is disabled: In most of my tests, tinyDTLS was unable to finish the handshake before the timeout due the first Client Hello record was never sent. However, on those occasions where the DTLS channel was established, the PS output marked that the 2048 bytes for wakaama client were used. Not big surprise as the master branch is still using tinyDTLS with malloc. |
|
Now that PR #7651 is merged I think this is a good moment for squashing and/or rebasing with the current master branch. Just some suggestions:
|
8d30637 to
21c0320
Compare
In what way? The function of that code is to re-open a DTLS connection after a certain time of inactivity in case the node is behind a NAT and the mapping of that connection timed out, meaning the DTLS server can't find that connection anymore if the new mapping is different. Since not everyone uses NAT and it's not even a factor in a RIOT application that only supports IPv6, I don't think this should be enabled by default. This is also not required as a sort of keep alive, since LWM2M clients have to update their registration with their servers before they time out (which is 300 seconds here, as defined by |
Fair enough |
|
I'm not going to have more time in the following months for checking this PR. The use of the tinyDTLS package with this PR is OK. It will be benefited from PR #7615 but is not required. And because memarray is already on master, this PR can be considered a candidate for merging. Still, I'm putting here a series of issues of interest for future reviewers. Non-blocking issues:
Blocking issues:
@moenoel, I think you can squash your commits now to make more easy the work of other reviewers. However, I'll suggest the following: |
07cb86a to
ebd7102
Compare
Thank you for the time you took so far to help me get this working 😃
This has not been the case for a while now. If there is no 'coaps://' uri in
Done. I also squashed support for bool sensor support that I recently added into the sensor support commit. |
|
I managed to reproduce the DTLS timing issue on a kw41z-mini. Adding a slight delay before sending packets seems to fix this for now (see 3ebaa9e). The kw41z-mini also ran into sporadic hardfaults due to misaligned memory in the memarray blocks. This can be fixed by either a569a30 or a02d1c2. Not sure which is better. Not using attributes seems cleaner to me, but using |
|
Another problem I was recently made aware of, is that Wakaama stores the URI without |
|
#7615 was merged. |
34b5e4e to
9fb7619
Compare
|
Just rebased on the current master (and apparently broke past squashes in the process? dunno how that happened; edit: I just re-squashed the history). Testing with the #7615 TinyDTLS looks good to me and the broken handshake should now also be fixed (see ebd8594). Turns out that |
* Bumps pkg/wakaama version * Fixes include path in pkg/wakaama/Makefile.include * Adds patches to fix warning in pkg/wakaama example code * Adds patches to pack all structs in pkg/wakaama * Makefile copies some code from wakaama client example * Ported wakaama/examples/client to RIOT's Sock API * Uses memarray for memory allocation
* Supports basically all sensors that return a float value * Can auto-add all sensors in the SAUL registry * Not tested with real sensors as of yet
9fb7619 to
0548d23
Compare
|
hi @moenoel do you plan to continue working on this PR? |
|
@jia200x That kinda depends on how much needs to be done for this to become merge-worthy. As I worked on this as part of a student project that has since ended, I might not have the time to finish this if there are too many open problems. Also, big part of the test environment/hardware came from other members of the project, so I won't have a proper test environment if I continue work on this. |
|
Hi @moenoel I checked in details this PR and it's awesome! Thanks for your contribution. Would love to see these features merged soon-ish ;)
If helps and you think is reasonable, this PR could be separated in several PRs to make it easier to review/merge. I would do it e.g like:
We did the same with the Openthread update (#7149 that was finally merged as #9336 ) and it got into master faster. Do you think is reasonable?
I see. In case you don't have time or don't plan to continue, I'm could give it a try to finish the adoption. If you have time, count me in as one of the reviewers/testers ;) We also have some hardware here to test. |
|
+1 on bringing in an LwM2M implementation. I also would be happy to review! |
In that case, I'd be happy to let you take the reigns @jia200x . I'd also be available to answer questions about the work I've done and give any support I can from my end. |
|
ok, thank you @moenoel! |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions. |
Contribution description
This pull request is a followup to #8148 and contains the following changes:
GNRCsock APIThe big change is obviously the port of the example client, which supports bootstrapping, DTLS (depends on TinyDTLS) and comes with the mandatory LwM2M objects/resources, as defined in the LwM2M specification (server, security and access control and device).
I tested this with with the Eclipse Leshan demo server and demo bootstrap server with and without DTLS (see readme file for instructions). Currently only PSK is supported/tested for DTLS.
As I currently don't have a board to test with, I only ran it on native so far.Tested on native, stm32f4discovery, pba-d-01-kw2x and kw41z-mini.
I've reused
BOARD_BLACKLISTandBOARD_INSUFFICIENT_MEMORYfromexamples/dtls-echosince I'm not certain how to (easily) determine which boards have enough resources to run this (especially since Wakaama makes heavy use ofthis is not an issue anymore, since moving to memarray (#7651). Though TinyDTLS still usesmalloc()malloc()).Known issues
Objects that are "too big" can't be requested whole, as the buffers are not large enough. Increasing
DTLS_MAX_BUFin the Makefile orRCVD_SIZE(depending on whether DTLS is enabled) inlwm2mclient.ccan fix this.When a NAT timeout is set (
DTLS_NAT_TIMEOUT > 0inconnection.h), the re-handshake will sometimes fail.When
LWM2M_WITH_LOGSis defined, some boards (e.g. iotlab-m3) die from a stack overflow.Possible timer issues resulting in DTLS handshake failures.
Possible next steps for this are:
Add ECC support for DTLSIssues/PRs references
Depends on
#7651(Pinging @cladmi as per #8106 and @florian-jean who might also be interested in this.)