Skip to content

Conversation

@mike-scott
Copy link
Contributor

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 mike-scott requested a review from rbtchc October 6, 2017 06:32
@mike-scott mike-scott changed the title LwM2M 1.10 changes: Add support for firmware push/pull + bugfixes LwM2M 1.10 changes: firmware support + bugfixes + more Oct 6, 2017
Copy link
Contributor

Choose a reason for hiding this comment

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

Per @rsalveti 's review, this will prevent application to use original http parser.
Therefore, maybe we should alter this to

depends on (HTTP_PARSER || HTTP_PARSER_URL_ONLY)

And set CONFIG_HTTP_PARSER_URL_ONLY=y in prj.conf

Copy link
Contributor

Choose a reason for hiding this comment

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

move the declaration of "uri_path" to the beginning of the function

Copy link
Contributor

Choose a reason for hiding this comment

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

Can be optimized at compile time if we change strlen(x) -> sizeof(x)-1

Copy link
Contributor

Choose a reason for hiding this comment

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

Line 398 ~ 447 can be removed and instead we can pass host URI string to net_app_init_udp_client()

@mike-scott
Copy link
Contributor Author

Posted V2 addressing comments from @rbtchc : https://hastebin.com/volejidodo.diff

@mike-scott
Copy link
Contributor Author

I'll fix the SHIPPABLE complaint this weekend.

Copy link
Contributor

Choose a reason for hiding this comment

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

We have to restore the server_addr after calling net_app_init_udp_client()

@mike-scott
Copy link
Contributor Author

V3 is rebased on master and includes a small change to restore the value in server_addr after net_app_init_udp_client() call.
Diff (ignoring master branch changes): https://hastebin.com/yugabukeke.m

NOTE: I get a ton of depreciated warnings when compiling. Hopefully SHIPPABLE doesn't break on those. My next patchset after this will address the CoAP API change.

rbtchc and others added 15 commits October 9, 2017 15:07
Add HTTP_PARSER_URL_ONLY to separate URL only parser

Signed-off-by: Robert Chou <[email protected]>
1. Parse firmware pull URI
2. Add lwm2m_firmware_get/set_update_cb() for applicaiton 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 trasnition
   as well as the sanity check

Signed-off-by: Robert Chou <[email protected]>
[[email protected]: rebased on net_app framework and
lwm2m_message refactoring.]
Signed-off-by: Michael Scott <[email protected]>
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 <[email protected]>
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 btw lwm2m_engine
   and lwm2m_obj_firmware_pull

Signed-off-by: Robert Chou <[email protected]>
[[email protected]: rebased on LwM2M net_app changes.]
Signed-off-by: Michael Scott <[email protected]>
Fix wrong check of DELIVERY_METHOD_PUSH to DELIVERY_METHOD_PUSH_ONLY

Signed-off-by: Michael Scott <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
[[email protected]: rebased on net_app + lwm2m_message
refactoring + firmware update changes.]
Signed-off-by: Michael Scott <[email protected]>
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 <[email protected]>
@mike-scott
Copy link
Contributor Author

Posted V4 fixing a compile break when CONFIG_LWM2M_FIRMWARE_UPDATE_PULL_SUPPORT wasn't enabled.
Diff: https://hastebin.com/udesibonun.cs

extern "C" {
#endif

#if !defined(CONFIG_HTTP_PARSER_URL_ONLY)
Copy link
Member

Choose a reason for hiding this comment

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

i remember having seen this change a while back wanted to comment on it but somehow forgot, dont really like all of this ifdefing, it would cleaner if we split the url parser out into its own library instead and reused it in the http library and elsewhere. All of those #ifs make the code and header unreadable

@mike-scott
Copy link
Contributor Author

This PR seems to be broken atm. I'm updating the source branch and it's not reflecting here. Going to try closing / reopening as I don't want to lose the comments.

@jukkar
Copy link
Member

jukkar commented Oct 17, 2017

This PR seems to be broken atm. I'm updating the source branch and it's not reflecting here. Going to try closing / reopening as I don't want to lose the comments.

Same thing happened to me on #4356 few hours ago. I resolved that by resending my branch and waiting for a while. It looked to me that there was glitch in github that solved itself by just waiting for few more minutes.

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.

5 participants