Skip to content

nanocoap: fix byte order of request message ID, and add test application - #7162

Merged
kaspar030 merged 2 commits into
RIOT-OS:masterfrom
kb2ma:nanocoap/msgid_byte_order
Jan 17, 2018
Merged

nanocoap: fix byte order of request message ID, and add test application#7162
kaspar030 merged 2 commits into
RIOT-OS:masterfrom
kb2ma:nanocoap/msgid_byte_order

Conversation

@kb2ma

@kb2ma kb2ma commented Jun 9, 2017

Copy link
Copy Markdown
Member

nanocoap was inconsistent in its public interface to the message ID for a request. For details, see the issue and the PR for the fix. This PR updates RIOT's package to include the fix. I also added a gcoap unit test to verify.

This PR uses the nanocoap from my fork, and I hope/expect @kaspar030 will accept the fix into his repository. Until that happens, RIOT can use the commits from the kb2ma nanocoap (sock) repository.

@aabadie aabadie added Area: network Area: Networking Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation labels Jun 23, 2017
@aabadie aabadie added this to the Release 2017.07 milestone Jun 23, 2017
@smlng

smlng commented Jan 9, 2018

Copy link
Copy Markdown
Member

nanocoap is now part of RIOT, @kb2ma please adapt this accordingly. The changes you requested on nanocoap (use htons) are not in RIOT yet, hence open a separate PR for that.

@kb2ma
kb2ma force-pushed the nanocoap/msgid_byte_order branch from ad6387d to b3d77ef Compare January 12, 2018 10:55
@kb2ma
kb2ma force-pushed the nanocoap/msgid_byte_order branch from b3d77ef to 7e5d14f Compare January 12, 2018 11:00
@kb2ma

kb2ma commented Jan 12, 2018

Copy link
Copy Markdown
Member Author

Adapted to nanocoap in RIOT. Created nanocoap unit test suite. Also, it turns out the byteorder function renaming was merged in #6897, so I think we're good there.

@aabadie aabadie added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 12, 2018
@smlng
smlng requested a review from kaspar030 January 12, 2018 11:31

@smlng smlng left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looks good, ACK!

@smlng
smlng removed the request for review from kaspar030 January 12, 2018 11:36
@smlng smlng added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jan 15, 2018
@smlng

smlng commented Jan 15, 2018

Copy link
Copy Markdown
Member

@kaspar030 any comments on this from your side?

@smlng smlng changed the title pkg/nanocoap: Update for fix to byte order of request message ID nanocoap: fix byte order of request message ID, and add test application Jan 17, 2018
@smlng

smlng commented Jan 17, 2018

Copy link
Copy Markdown
Member

ping @kaspar030, this should go into to the release, too.

@kaspar030 kaspar030 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ACK.

@kaspar030
kaspar030 merged commit 3f99048 into RIOT-OS:master Jan 17, 2018
@kb2ma
kb2ma deleted the nanocoap/msgid_byte_order branch February 4, 2019 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants