Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LwM2M 1.10 changes: firmware support + bugfixes + more (2nd version) #4380

Merged
merged 15 commits into from
Oct 17, 2017

Conversation

mike-scott
Copy link
Contributor

NOTE: Original PR (#4208) was corrupted and stopped responding to updates to the base branch in my repo. This is a new version of that PR.

This patchset does the following:

support for firmware handling: including push and pull methods
moves some SYS_LOG message from DBG to INF for better visibility
couple of bugfixes which leaked packets
retry handling for firmware downloads
application callbacks from the registration client

@mike-scott
Copy link
Contributor Author

Rebased against current master. This PR is essentially #4208 except that the first commit was reworked.

The relevant diff from #4208 is here: https://hastebin.com/aluqigaker.hs

@mike-scott mike-scott force-pushed the lwm2m-fixes branch 2 times, most recently from 1b849bc to c5c104f Compare October 17, 2017 16:15
@SebastianBoe
Copy link
Collaborator

@mike-scott : Seems github is having some issues at the moment.

https://status.github.com/messages

@mike-scott
Copy link
Contributor Author

@SebastianBoe Thank you! I was still looking to see if something on my end was broken :)

@mike-scott mike-scott closed this Oct 17, 2017
@mike-scott mike-scott reopened this Oct 17, 2017
@@ -122,9 +122,15 @@ int lwm2m_device_add_err(u8_t error_code);
#define RESULT_UPDATE_FAILED 8
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typos in commit message
s/applicaiton/application/
s/trasnition/transition/

for (i = 0; i < len; i++) {
if (firmware_uri[off + i] == '/') {
if (path_len > 0) {
ret = zoap_add_option(&msg->zpkt,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we use the new coap API here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New CoAP API changes are coming after this patchset.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jukkar for reference here's my current patch queue for LwM2M / CoAP / DTLS (based on master): https://github.com/mike-scott/zephyr/commits/master-lwm2m

This patchset ends near the bottom of the first page with "net: lwm2m: add RD client callbacks for app".

@@ -123,10 +123,10 @@ int lwm2m_device_add_err(u8_t error_code);
#define RESULT_UNSUP_PROTO 9
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a typo in commit message s/btw/between/ ?

@@ -162,6 +162,7 @@ do_firmware_transfer_reply_cb(const struct zoap_packet *response,
struct zoap_packet *check_response = (struct zoap_packet *)response;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in commit msg s/Both/both/

/* test for duplicate transfer */
if (firmware_block_ctx.current < received_block_ctx.current) {
SYS_LOG_WRN("Duplicate packet ignored");
/* restore main firmware block context */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps empty line before comment

@mike-scott
Copy link
Contributor Author

Will address @jukkar comments. Thanks for the review!

@mike-scott
Copy link
Contributor Author

Updated commit descriptions per @jukkar and here's the diff for the spacing change: https://hastebin.com/tajiqadumo.swift

@mike-scott
Copy link
Contributor Author

Quick Update: Found a left-over CONFIG name in the LwM2M Kconfig
Diff: https://hastebin.com/onegegacov.vbs

@nashif
Copy link
Member

nashif commented Oct 17, 2017

just fix that commit message line length...

Michael Scott and others added 8 commits October 17, 2017 16:03
With future patches we will need to parse URLs in the registration
client and firmware object.  Enable it by default when LWM2M is
enabled.

Signed-off-by: Michael Scott <michael.scott@linaro.org>
1. Parse firmware pull URI
2. Add lwm2m_firmware_get/set_update_cb() for application to register
   callback. This is because we want to check the update_state before
   we pass to the application
3. Add lwm2m_firmware_get/set_update_result() and
   lwm2m_firmware_get/set_update_stat() to manage the state transition
   as well as the sanity check

Signed-off-by: Robert Chou <robert.ch.chou@acer.com>
[michael.scott@linaro.org: rebased on net_app framework and
lwm2m_message refactoring.]
Signed-off-by: Michael Scott <michael.scott@linaro.org>
OPAQUE resource type might/might not have data_ptr/data_len setup
depending on the implementation. This introduce an issue that when
OPAQUE resource is written from the server side, the ones w/ none
setup will not be able to get the data at post_write_cb()

Modify to setup data_ptr/data_len as incoming buffer and buffer size

Signed-off-by: Robert Chou <robert.ch.chou@acer.com>
1. Add handling block1 option in handle_request(). The basic idea is
   to declare structure block_context at compiled time and use "token"
   as a key to pick up the on-going block cotext. It should be able to
   support multiple blockwise transfer concurrently
2. Use write callback implemented in lwm2m_obj_firmware to deal w/ the
   update state transition and than call the callback registered by the
   application
3. move default_block_size to lwm2m_engine.c to share between
   lwm2m_engine and lwm2m_obj_firmware_pull

Signed-off-by: Robert Chou <robert.ch.chou@acer.com>
[michael.scott@linaro.org: rebased on LwM2M net_app changes.]
Signed-off-by: Michael Scott <michael.scott@linaro.org>
Fix wrong check of DELIVERY_METHOD_PUSH to DELIVERY_METHOD_PUSH_ONLY

Signed-off-by: Michael Scott <michael.scott@linaro.org>
This is a useful message announcing that the RD client state machine
is starting for a particular connection.  If the log level is set
low so that DBG messages are hidden, then this message goes away.

Signed-off-by: Michael Scott <michael.scott@linaro.org>
Signed-off-by: Michael Scott <michael.scott@linaro.org>
In the case of a proxy server translating HTTP -> COAP (known in
the code as "separate reply"), we were leaking lwm2m_message structures.
This was due to pending objects being cleared out during the first ACK,
and no other way of finding a matching message when the follow up packet
was received.  Let's add a second match for reply to make sure we can
find our matching message resources.

NOTE: This change renames find_msg_from_pending() to find_msg() and
makes it a static function as it's only used by the lwm2m_engine.

Signed-off-by: Michael Scott <michael.scott@linaro.org>
Michael Scott and others added 7 commits October 17, 2017 16:03
Create an internal function lwm2m_engine_context_init() which sets
the extra packet pools and initializes retransmit work internal to
the LwM2M engine.

This function will be used by firmware pull support which establishes
a new LwM2M context for downloading firmware.

Signed-off-by: Michael Scott <michael.scott@linaro.org>
Previously, firmware support wasn't initializing the retransmit work
or the extra network packet pools.  Let's fix that.

NOTE: While this fixes the setup of retransmit work, the actual
attempts to re-send packets which are pending is failing.  Needs
another follow-up fix.

Signed-off-by: Michael Scott <michael.scott@linaro.org>
When a packet expires after the pending retries we call
lwm2m_release_message() to free up resources.  This includes
cleanup of the pending structure which calls net_pkt_unref on
the pending packet.  This would normally free up the packet
memory.  However, earlier in the pending processing we add a ref
to the packet so that normal send processing doesn't free up
the memory.   This meant we were leaking packet memory every
time we had an expiration due to timeout.

Let's do an unref prior to calling lwm2m_release_message() to
make sure the packet memory is freed correctly.

Signed-off-by: Michael Scott <michael.scott@linaro.org>
UDP packets can be lost in heavy traffic.  Normally we can handle this
with pending packet processing for packets which have not been responded
to with an ACK.  However, due to the time it takes for firmware to
download via CoAP, an extra level of retries should be added.

The process works like this:

Normal pending packets will try to send 3 times fairly quickly.
If that fails, then the timeout callback is called for the firmware
download process.  A retry counter is incremented and the timeout
callback perform a new packet send of the block-wise transfer
packet that is missing, until the retry counter hits a limit (3)
and then the transfer is aborted.

This allows for a longer "outage" to happen during firmware transfer
and the process can still succeed.

NOTE: This patch does not fix a current bug where the pending process
is not re-sending the packets correctly, it only makes the process
more stable with a better chance to work.

Signed-off-by: Michael Scott <michael.scott@linaro.org>
During firmware download via block-wise transfer, we can see
packets occaionally get re-transmitted (normal logic in the
pending / retry functions).  However, both of these packets
end up coming through the reply handler and we should ignore
any block-wise transfer that has a current value less than
where we expect to be.

NOTE: This fixes K64F ethernet transfers where we were getting
too many packets back in the handler.

Signed-off-by: Michael Scott <michael.scott@linaro.org>
CoAP allows a proxy to be used when transferring data (CoAP-CoAP and/or
CoAP-HTTP) by creating request on a specific URI path and by using the
Proxy URI CoAP option. Create specific Kconfig options for the proxy
server address and port, until a parser gets implemented.

Code tested with Californium acting as CoAP proxy.

Signed-off-by: Ricardo Salveti <ricardo.salveti@linaro.org>
[michael.scott@linaro.org: rebased on net_app + lwm2m_message
refactoring + firmware update changes.]
Signed-off-by: Michael Scott <michael.scott@linaro.org>
Applications may want to be notified when various events
happen in the LwM2M rd client.  Let's implement an event
callback which sends: connect, disconnect and update events.

Signed-off-by: Michael Scott <michael.scott@linaro.org>
@mike-scott
Copy link
Contributor Author

@nashif Woops! Re-pushed with the commit description line length fixed. Also, double checked checkpatch on the other patches as well. SHIPPABLE should pass now.

@mike-scott
Copy link
Contributor Author

Ok all looks good now. :)

@nashif nashif merged commit 1965e5e into zephyrproject-rtos:master Oct 17, 2017
@mike-scott mike-scott deleted the lwm2m-fixes branch October 20, 2017 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants