Skip to content

Conversation

@satishgn
Copy link
Contributor

@satishgn satishgn commented Aug 1, 2014

Need cloud-server code additions required to get this feature working.
New CoAPCode::POST path is 's' for SAVE_BEGIN.
Mandatory arguments required : flash_address and file_size

@dmiddlecamp
Copy link
Contributor

awesome, that should be no problem. :)

@m-mcgowan
Copy link

Hi @satishgn - With the addition of SAVE_BEGIN to the CoAPMessageType enum, the enum values declared after will have changed value - I'm guessing these are only used on the client?

There are some points that I hope you could clarify in documentation:

  • what are responsibilities of the callback? For example should it validate the address/length and what happens if these are out of range?
  • what are the semantics of address - is it an absolute flash address or relative to the start of the OTA flash region? (While both are equivalent now, it may not always be like that.)

Could you also extend the existing test suite to verify the new code added?

If you need clarification or assistance with any of this just let me know.

PS: One more thing, if it's possible for you to rebase with current master, that will provide the .travis.yml so that the continuous integration server can run the test code.

@satishgn
Copy link
Contributor Author

satishgn commented Sep 1, 2014

Hi @m-mcgowan , SAVE_BEGIN is used locally so should not affect the existing code flow.

  • the validation of the external flash address and file length can happen on the device-server and/or core-firmware. Better to avoid validation in core-communication-lib.
  • for address, it is the absolute flash address and preferably in multiples of sFLASH_PAGESIZE(0x1000)

Probably I would need some help in extending the existing test suit and an idea on how to verify the test. how to use travis?

Have to rebase master in all the repos:
core-common-lib/flasher-additions
core-communication-lib/flasher-additions
core-firmware/flasher-additions

@satishgn
Copy link
Contributor Author

satishgn commented Sep 2, 2014

Merged in latest master changes

Dependencies:
particle-iot-archived/core-common-lib#32

@m-mcgowan
Copy link

Cool, thanks for rebasing - now you see we get a test passed above. Now we know the new code hasn't broken anything.

I agree about not validating the calls here - it's the callback's job to do that, but there wasn't much documentation to explain what values the callback should expect. But missing doc comments is not anything new, none of the callbacks have any docs!

@m-mcgowan
Copy link

closing - already present in feature/hal in the combined repo.

@m-mcgowan m-mcgowan closed this Apr 3, 2015
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.

4 participants