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

block2 request #27

Merged
merged 21 commits into from
Jun 17, 2014
Merged

block2 request #27

merged 21 commits into from
Jun 17, 2014

Conversation

nqd
Copy link
Collaborator

@nqd nqd commented Jun 8, 2014

coap.js now can request for a resource that return in blockwise(2).

Tested with coap.request('coap://coap.me/.well-known/core').

Todo

  • block2 for observe
  • block1

@mcollina
Copy link
Collaborator

mcollina commented Jun 8, 2014

Super cool!

However, could you please add unit tests? I can't merge it without them, otherwise I might broke things :/

num = (block2Value[0]*256*256 + block2Value[1]*256 + block2Value[2]) >>4
break
default:
throw new Error('Too long block2 option size: '+block2Value.length)
Copy link
Collaborator

Choose a reason for hiding this comment

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

please, do no throw, but emit an error instead.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, will update

@mcollina
Copy link
Collaborator

Woow! This is so much work! Thank you very much!

I see a failing test on Travis, can you please check?

@nqd
Copy link
Collaborator Author

nqd commented Jun 17, 2014

My local testing success, but travis one fail. I'm investigating!

@nqd
Copy link
Collaborator Author

nqd commented Jun 17, 2014

Could you please accept my pull request for coap packet. This is a simple commit which declares block1 and block2 options. After updating this coap-packet, the travis test will be passed.

For this pull request, some stuffs are added:

  • coap server can return with blockwise (2) for big message. Max payload is declared at parameters p.maxPacketSize = 1280.
  • coap client can make early negotiation.
  • some tests

Todo:

  • blockwise for observe
  • block1

"lru-cache": "~2.5.0"
"coap-packet": "~0.1.8",
"lru-cache": "~2.5.0",
"underscore": "^1.6.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is underscore still needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, I forgot to delete it!

catch (e) {
req.sender.reset()
return req.emit('error', err)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do not try catch, see explanation down there.

@mcollina
Copy link
Collaborator

Great, with the new coap-packet the Travis build passes!

@mcollina
Copy link
Collaborator

Anyway, do you want to help maintaining/developing this library according to https://github.com/mcollina/node-coap/blob/master/CONTRIBUTING.md?

@nqd
Copy link
Collaborator Author

nqd commented Jun 17, 2014

Yes, with all my pleasure for my first "contributing" to open source.
Working close to sensor network, I will go around coap.js for a while.

@mcollina
Copy link
Collaborator

Feel free to add yourself to the README :).

@nqd nqd merged commit 199c9fe into coapjs:master Jun 17, 2014
@mcollina
Copy link
Collaborator

Can you please add an example of a Blockwise request and server in the examples/ folder?

@nqd
Copy link
Collaborator Author

nqd commented Jun 17, 2014

Blockwise examples will be added soon!

On Thứ ba, 17 Tháng sáu Năm 2014 23:42:11 ICT, Matteo Collina wrote:

Can you please add an example of a Blockwise request and server in the
examples/ folder?


Reply to this email directly or view it on GitHub
#27 (comment).

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.

2 participants