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

Support Bootstrap-Read and Bootstrap-Discover #549

Merged

Conversation

sbertin-telular
Copy link
Contributor

Signed-off-by: Scott Bertin sbertin@telular.com

@qleisan
Copy link

qleisan commented Mar 4, 2021

Starting to review (and read up on spec....). Expect to be done later today

Copy link

@qleisan qleisan left a comment

Choose a reason for hiding this comment

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

Code-wise I have no objections but I struggle a bit with the bigger picture (how this is used/triggered). I believe this code is already tested and "in production". (Right?) I'll approve so it can be merged but will continue to read up and do hands on tests

* or internals.h LWM2M_MAX_PACKET_SIZE!
*/
#define MAX_PACKET_SIZE 198
#define MAX_PACKET_SIZE 1024
Copy link

Choose a reason for hiding this comment

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

MAX_PACKET_SIZE in client/server is 2048. Better to be consistant?

@qleisan
Copy link

qleisan commented Mar 5, 2021

210305_PR549_test1.pcapng.zip
210305_PR549_test2.pcapng.zip

Found blocktransfer related issue while testing Bootstrap-Discover
modified bootstrap_server.ini as follows:

# For any client, we delete all server accounts and
# provision the Leshan server info
[Endpoint]
Delete=/0
Delete=/1
Server=1
Discover=/

launched example bootstrap server, server and client

./build-bootstrap/bootstrap_server -f examples/bootstrap_server/bootstrap_server.ini -4
./build-server/lwm2mserver -4
./build-client/lwm2mclient -4 -b -n bepa2

With REST_MAX_CHUNK_SIZE set to 128 (current setting) the result was according to test2 - ERROR
With REST_MAX_CHUNK_SIZE set to 512 (modified setting) the result was according to test1 - OK

also it would be helpful for the end user with some updated comments in the .ini file about the new command(s)

@sbernard31
Copy link
Contributor

About 210305_PR549_test2.pcapng.zip this looks like #536, as Wakaama answers to a response with a response (See MID 55386).

I see that Bootstrap server try several time to send the discover request ? does it go in an infinite loop or the retry we see are just retransmission ?

@sbertin-telular
Copy link
Contributor Author

I'm not using these features in production. This was to make Wakaama more feature complete.

I'll update MAX_PACKET_SIZE and add some comments to the .ini file.

@sbertin-telular
Copy link
Contributor Author

The retries look like CoAP retries and are limited to 5 total.

I'll squash the changes once everything is good.

Signed-off-by: Scott Bertin <sbertin@telular.com>
@sbertin-telular
Copy link
Contributor Author

Squashed

@sbernard31 sbernard31 merged commit 22a5eb4 into eclipse-wakaama:master Mar 8, 2021
@sbernard31
Copy link
Contributor

@qleisan, @tuve about #549 (comment) we need maybe a dedicated issue to track this ?

@qleisan
Copy link

qleisan commented Mar 8, 2021

created #551

@sbertin-telular sbertin-telular deleted the bootstrap_read_rebase branch March 8, 2021 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants