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

Introduce IDE/PlatformIO; add wolfSSL Benchmark and Test Examples #7528

Merged
merged 5 commits into from
May 17, 2024

Conversation

gojimmypi
Copy link
Contributor

Description

Adds IDE/PlatformIO examples.

See also #7413 and platformio/platformio-registry#85

Fixes zd# n/a

Testing

How did you test?

Tested manually using PlatformIO in VS Code.

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

Copy link
Contributor

@bandi13 bandi13 left a comment

Choose a reason for hiding this comment

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

Tested both Benchmark as well as Test examples. Benchmark works, Test needs to be fixed.

@gojimmypi
Copy link
Contributor Author

@bandi13 thanks for testing this for a test drive!

It's actually a good thing that the example test app is failing here. The published wolfSSL 5.7.0-rev.3b (Post Release Update for PlatformIO) referenced by the example on PlatformIO is stale.

I introduced a SHA interleave test in #7262. This test is included in the stale, post 5.7.0 version published at PlatformIO and is failing as desired, properly detecting the interleave problem.

The failing SHA interleave was the root cause of the SRP errors noted in #7210.

The SHA/SRP problem was fixed in #7505, merged within the last day but not yet published to PlatformIO.

Unfortunately (or fortunately, depending on your perspective), I found a new problem with the latest version of wolfSSL for PlatformIO as noted in #7533.

My plan is to create a new PR to fix #7533 and then publish a new, post-release wolfSSL to PlatformIO.

I don't think there's any problem with this particular PR 7528, as the root cause is the currently published wolfSSL version at PlatformIO. I've confirmed the latest master branch of wolfSSL works properly for both the SHA and SRP tests.

Reminder the SRP for the Apple HomeKit also requires the FP_MAX_BITS setting:

#define WOLFCRYPT_HAVE_SRP
#define FP_MAX_BITS (8192 * 2)

See the 4 different versions of wolfSSL available at PlatformIO:

Note the Arduino version of the PlatformIO library should not be confused with the Official wolfSSL for Arduino: https://github.com/wolfSSL/Arduino-wolfSSL which is published to https://www.arduino.cc/reference/en/libraries/wolfssl/

This initial rollout has been a little more bumpy than anticipated. I do believe that once the appropriate platform-specific fixes are implemented, that the regular release cycle should be considerably more stable and graceful.

@dgarske
Copy link
Contributor

dgarske commented May 15, 2024

@gojimmypi do you want to rebase the PR or accept as-is?

@dgarske dgarske requested a review from bandi13 May 15, 2024 16:07
dgarske
dgarske previously approved these changes May 15, 2024
@gojimmypi
Copy link
Contributor Author

@dgarske please stand by. I'd like to rebase and apply some minor updates. Testing is taking longer than anticipated.

@gojimmypi
Copy link
Contributor Author

Hi @dgarske I've refreshed from upstream and made some minor changes in 503bbbe.

Key to getting this to work was having the proper fresh update of wolfSSL published to PlatformIO.

Tested and confirmed working are these versions in platformio.ini:

@bandi13 if you could please, take this for a test drive again & let me know how it goes.

@gojimmypi
Copy link
Contributor Author

Jenkins retest this please.

@@ -0,0 +1,791 @@
/* examples/configs/user_settings_platformio.h
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this new file user_settings_platformio.h missing from include.am?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Updated this, the respective README, and changed sort order.

@@ -0,0 +1,24 @@
#include <wolfssl/wolfcrypt/settings.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

If possible please add our standard GPL header

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@gojimmypi
Copy link
Contributor Author

Jenkins retest this please

@dgarske dgarske assigned wolfSSL-Bot and bandi13 and unassigned dgarske and gojimmypi May 16, 2024
Copy link
Contributor

@bandi13 bandi13 left a comment

Choose a reason for hiding this comment

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

All tests pass. Benchmarks also run on an ESP32.

Nice work!

@dgarske dgarske merged commit 7782f8e into wolfSSL:master May 17, 2024
103 checks passed
jefferyq2 pushed a commit to jefferyq2/wolfssl that referenced this pull request Jun 9, 2024
Introduce IDE/PlatformIO; add wolfSSL Benchmark and Test Examples
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