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

Third-party dependency update #1864

Merged
merged 12 commits into from Aug 26, 2019

Conversation

@avtolstoy
Copy link
Member

commented Jul 26, 2019

Problem

DeviceOS depends on a number of third-party components, which we should strive to keep up to date in order to not be affected by various issues in them possibly fixed in the recent versions. We've had several of such occurances already.

Solution

This PR updates the following dependencies:

Dependency Current version Current commit New version New commit Changelog Particle changes
FreeRTOS 10.0.1 particle-iot/freertos@3feb84f 10.2.1 particle-iot/freertos@c4e1510 Changelog -
Catch2 2.4.2 particle-iot/Catch2@03d122a 2.9.1 particle-iot/Catch2@2f631bb Changelog -
FakeIt 2.0.5-7-g362271d particle-iot/FakeIt@362271d 2.0.5-13-g317419c https://github.com/particle-iot/fakeit/317419c2e2f5a98e023a4d62628eb149fe3d3c3a Changelog -
LittleFS 1.6.0 particle-iot/littlefs@9996634 1.7.2 particle-iot/littlefs@ed07f60 Changelog Diff
LwIP 2.0.5 particle-iot/lwip@362271d 2.1.2 particle-iot/lwip@4fe0495 Changelog Diff
LwIP Contrib 2.0.0 particle-iot/lwip-contrib@cce6cd1 2.1.2 particle-iot/lwip-contrib@35b011d Changelog -
MbedTLS 2.9.0 particle-iot/mbedtls@48fe5b1 2.16.2 particle-iot/mbedtls@4e4e631 Changelog Diff
miniz 2.0.7 particle-iot/miniz@f07041c 2.1.0 particle-iot/miniz@af839bf Changelog Diff
nanopb 0.3.9 particle-iot/nanopb@2d40a90 0.3.9.3 particle-iot/nanopb@6b91cc5 Changelog Diff
OpenThread 20190130 particle-iot/openthread@0eab4ec 20190709 particle-iot/openthread@1bb328f Changelog Diff
WizNet Ethernet Driver 20180515 particle-iot/ioLibrary_Driver@53401b1 20190529 particle-iot/ioLibrary_Driver@890a73f Changelog Diff

LittleFS

We haven't upgraded LittleFS to the latest stable 2.0.0 and are still on 1.x line, because the format of the filesystem has significantly changed and the devices will require to migrate to it. Given that both our bootloader and system firmware require access to the DCT (which is stored as a file in the filesystem) we'll need to be careful with this process, so we'll be evaluating the upgrade to 2.x line separately and will upgrade in a separate PR later on as well.

Steps to Test

Pretty much everything needs to be retested.

Example App

N/A

References

  • [CH29642]

Changelog

@m-mcgowan
Copy link
Contributor

left a comment

lgtm. Please document the steps used to upgrade 3rd party dependencies, in particular the dependencies that are not under the third_party folder. Some rationale for the changes in the concurrent_hal would be helpful

@@ -145,6 +145,7 @@ auto SessionPersist::restore(mbedtls_ssl_context* context, bool renegotiate, uin

mbedtls_ssl_handshake_wrapup(context);
size = sizeof(*this);

This comment has been minimized.

Copy link
@m-mcgowan

m-mcgowan Aug 11, 2019

Contributor

The rationale for whitespace here would be good to include in our style guide.

This comment has been minimized.

Copy link
@avtolstoy

avtolstoy Aug 12, 2019

Author Member

It's obviously an unintended change 😄 I'll revert it.

* while buffering multiple smaller handshake messages.
*
*/
#define MBEDTLS_SSL_DTLS_MAX_BUFFERING 16384

This comment has been minimized.

Copy link
@m-mcgowan

m-mcgowan Aug 11, 2019

Contributor

Please add a doc comment stating why this is less than the recommended value and how the value was derived.

This comment has been minimized.

Copy link
@avtolstoy

avtolstoy Aug 12, 2019

Author Member

I'll update the header, but for quick reference this is what we've been using with our own implementation of DTLS message queuing.

@@ -691,7 +622,8 @@ void OpenThreadNetif::refreshIpAddresses() {
addresses_[i].mValid = true;
addresses_[i].mPreferred = !memcmp(&config, &active, sizeof(config));
if (config.mRloc16 != ourRloc16) {
otIp6CreateRandomIid(ot_, &addresses_[i], nullptr);
Random rand;

This comment has been minimized.

Copy link
@m-mcgowan

m-mcgowan Aug 11, 2019

Contributor

What's the reason for this change?

This comment has been minimized.

Copy link
@avtolstoy

avtolstoy Aug 12, 2019

Author Member

otIp6CreateRandomIid is no longer available so we are emulating its behavior.

@@ -384,19 +380,19 @@ int os_mutex_destroy(os_mutex_t mutex)

int os_mutex_lock(os_mutex_t mutex)
{
xSemaphoreTake(mutex, portMAX_DELAY);
xSemaphoreTake(reinterpret_cast<QueueHandle_t>(mutex), portMAX_DELAY);

This comment has been minimized.

Copy link
@m-mcgowan

m-mcgowan Aug 11, 2019

Contributor

SemaphoreHandle_t?

This comment has been minimized.

Copy link
@avtolstoy

avtolstoy Aug 12, 2019

Author Member

SemaphoreHandle_t is just an alias to QueueHandle_t. FreeRTOS implements almost all of the synchronization primitives on top of the queue. However I agree, it would make sense to use specific types for the sake of clarity and to avoid having to make changes if this ever changes. 👍

return error;
}

bool otPlatRadioIsEnabled(otInstance *aInstance)

This comment has been minimized.

Copy link
@m-mcgowan

m-mcgowan Aug 11, 2019

Contributor

Is this code that we maintain? if not, it would be clearer to have it in third_party modules.

This comment has been minimized.

Copy link
@avtolstoy

avtolstoy Aug 12, 2019

Author Member

These sources come (modified) from OpenThread examples folder. I'd rather not modify the original example sources within our OpenThread fork and unless we create a separate repository just so that we could have them in third_party folder, I'd prefer to keep things as-is. We have other cases like this as well: e.g. hal/src/nRF52840/mbedtls/. I'm open to other suggestions to improve this.

@avtolstoy

This comment has been minimized.

Copy link
Member Author

commented Aug 12, 2019

@m-mcgowan

Please document the steps used to upgrade 3rd party dependencies, in particular the dependencies that are not under the third_party folder.

I'll create a separate PR targeting the documentation.

Some rationale for the changes in the concurrent_hal would be helpful

As for this one, I doubt the changes I've done warrant documentation in our source code. It was wrong of us to assume that FreeRTOS-specific types (e.g. QueueHandle_t) are to always stay as typedefs to void*. They are not anymore: https://github.com/particle-iot/freertos/blob/master/FreeRTOS/Source/include/queue.h#L48

@avtolstoy avtolstoy force-pushed the feature/third-party-update branch from b75df8e to cc694c1 Aug 23, 2019

@avtolstoy

This comment has been minimized.

Copy link
Member Author

commented Aug 23, 2019

Review comments addressed.

@avtolstoy avtolstoy modified the milestones: 1.4.0, 1.4.0-rc.1 Aug 23, 2019

@avtolstoy avtolstoy merged commit cae0c07 into develop Aug 26, 2019

2 of 3 checks passed

Codacy/PR Quality Review Not up to standards. This pull request quality could be better.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@avtolstoy avtolstoy deleted the feature/third-party-update branch Aug 26, 2019

@kubark42 kubark42 referenced this pull request Sep 9, 2019
3 of 6 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.