Espressif Managed Component wolfSSH 1.4.18 post-release update #770
Conversation
|
I'll probably be converting to draft while I investigate the test failures for my |
|
It appears the root cause of the original The I've added Recall that Managed Component files cannot be changed without reverting to a non-managed component configuration. This PR now only changes Espressif examples, along with the I am otherwise unable to reproduce the GitHub actions errors: I'm assuming the tests have been changed since the v1.4.18 that I am attempting. |
|
Could it be that the
resulting in: |
ide/include.am
Outdated
| include ide/CSBENCH/include.am | ||
| include ide/MQX/include.am | ||
| incldue ide/Espressif/include.am |
There was a problem hiding this comment.
This is what is breaking the builds, typo
There was a problem hiding this comment.
oh! thanks so much for that. fixed
…om/gojimmypi/wolfssh into pr-post-release-bbba8aef-espressif
|
Thanks @LinuxJedi for spotting the typo and @JacobBarthelmeh for confirming the remaining problem seems to be related to the base commit and (probably) not the contents of this PR. I'm ready for code review for the original PR description (edited several times), in particular the This latest change on this branch has been published to gojimmypi/mywolfssh version 1.4.18-preview1r |
|
|
||
| # The root of the project is two directories up from here. (we are typically in [project name]components/mywolfssh) | ||
| get_filename_component(PROJECT_ROOT "${THIS_DIR}" DIRECTORY) # Up one directory from here is "components" | ||
| get_filename_component(PROJECT_ROOT "${PROJECT_ROOT}" DIRECTORY) # up one more directory should be the root of our project |
There was a problem hiding this comment.
All of the above will likely break out-of-tree building. I'm assuming this doesn't matter for ESP?
There was a problem hiding this comment.
Good question. This is very Espressif-specific.
The relative paths do not change.
A "component" in the local project directory MUST be in the components subdirectory, directly under the main project. In the case of wolfssl, in the components/wolfssl directory. There's no flexibility here.
The name of a managed component is slightly different: managed_components
See the IDF Component Manager docs.
| endif() | ||
| message(STATUS "") | ||
|
|
||
| if ( "${${OUTPUT_FOUND_WOLFSSH_DIRECTORY}}" STREQUAL "" ) |
There was a problem hiding this comment.
I don't think you need quotes around the variable. You would normally do:
if ( OUTPUT_FOUND_WOLFSSH_DIRECTORY STREQUAL "" )Also, I've never seen double ${ } around a variable before. Is this some kind of reference to a variable or something like that?
There was a problem hiding this comment.
oh, very observant. This one is a bit tricky.
The OUTPUT_FOUND_WOLFSSH_DIRECTORY is an output parameter to a function called FIND_WOLFSSH_DIRECTORY
function(FIND_WOLFSSH_DIRECTORY OUTPUT_FOUND_WOLFSSH_DIRECTORY)
The parameter value is the name of the variable. See the call: FIND_WOLFSSH_DIRECTORY(WOLFSSH_ROOT)
In this case, OUTPUT_FOUND_WOLFSSH_DIRECTORY contains the variable name WOLFSSH_ROOT (not the actual path stored in WOLFSSH_ROOT
The reference to the variable WOLFSSH_ROOT contents will be updated inside the function. The double ${ } is needed for that.
| #error "Problem with wolfSSL user_settings. " \ | ||
| "Check components/wolfssl/include " \ | ||
| "and confirm WOLFSSL_USER_SETTINGS is defined, " \ | ||
| "typically in the component CMakeLists.txt" |
There was a problem hiding this comment.
The quote marks do not work how you would think they do here. Should be:
#error Problem with wolfSSL user_settings. \
Check components/wolfssl/include \
and confirm WOLFSSL_USER_SETTINGS is defined, \
typically in the component CMakeLists.txtThere was a problem hiding this comment.
gcc example with quotes. A best practice to use quotes, no?
https://gcc.gnu.org/onlinedocs/cpp/Diagnostics.html
It does work with or without quotes for Espressif, including the ESP8266 3.5 SDK
Do we have a standard? For example in tls13.
There was a problem hiding this comment.
It works until you do multiline. Then it looks weird. I guess it doesn't really matter.
There was a problem hiding this comment.
As an example:
void main(void)
{
#error "Problem with wolfSSL user_settings. " \
"Check components/wolfssl/include " \
"and confirm WOLFSSL_USER_SETTINGS is defined, " \
"typically in the component CMakeLists.txt"
}When compiling:
$ gcc error.c
error.c: In function ‘main’:
error.c:3:10: error: #error "Problem with wolfSSL user_settings. " "Check components/wolfssl/include " "and confirm WOLFSSL_USER_SETTINGS is defined, " "typically in the component CMakeLists.txt"
3 | #error "Problem with wolfSSL user_settings. " \
| ^~~~~| /* Define WOLFSSL_USER_SETTINGS project wide for settings.h to include */ | ||
| /* wolfSSL user settings in ./components/wolfssl/include/user_settings.h */ | ||
| #error "Missing WOLFSSL_USER_SETTINGS in CMakeLists or Makefile:\ | ||
| CFLAGS +=-DWOLFSSL_USER_SETTINGS" |
There was a problem hiding this comment.
As my previous comment with the quote marks.
| /* TODO determine low memory configuration for ECC. */ | ||
| #else | ||
| /* when you want to use SHA512 */ | ||
| #define WOLFSSL_SHA512 |
There was a problem hiding this comment.
Something not for this pull request, but to think about later, #767 adds HMAC-SHA2-512, it can be disabled at compile time. We should probably make sure that happens automatically with the 8266.
There was a problem hiding this comment.
oh, thanks for the heads up on that one. I've left myself a breadcrumb reminder.
| /* Define WOLFSSL_USER_SETTINGS project wide for settings.h to include */ | ||
| /* wolfSSL user settings in ./components/wolfssl/include/user_settings.h */ | ||
| #error "Missing WOLFSSL_USER_SETTINGS in CMakeLists or Makefile:\ | ||
| CFLAGS +=-DWOLFSSL_USER_SETTINGS" |
There was a problem hiding this comment.
As my previous comments with the quote marks in #error directives for this and the one above it.
|
Jenkins retest this please |
This PR includes some post-release wolfSSH v1.18 updates for publishing to the Espressif Component Registry. The update is applied to the
bbba8aef(v1.4.18 release). I'm also planning a v1.4.19 Managed Component Release in the near future.Testing was performed using mywolfssl and mywolfssh, both using my
gojimmypinamespace.Core wolfSSH change
There's only one minor modification to the core wolfSSH library:
test.hwas edited to include an exclusion criteria duringWOLFSSH_TEST_THREADING, specifically adding this!defined(SINGLE_THREADED):Otherwise, this error is encountered at compile time:
Espressif Examples Updated
Most of this PR is related to the Espressif examples:
user_settings.handCMakeLists.txtfrom the reference template example.CmakeLists.txtwith various improvements, particularly related to more robust "find source code", better support of environment variables forWOLFSSH_ROOT_, and improved diagnostic messages.wolfssh_echoserverexample now has improved settings for default SSID and password for WiFi setup.CMakeLists.txtfiles that were missingSTATUSinmessage()statements, which previously would be STDERR messages.ESP_ENABLE_WOLFSSHfrom CFLAGS and set related required settings in the respectiveuser_settings.h.Testing
Of particular interest during first build: the versions of components fetched. For example:
Template Example Output
SSH EchoServer Example Output
See the example README.md
Upon successful startup, look for the most recent lines
Starting wolfSSH server_test:Connect to the ESP32 IP address, port 22222, using the example shown IP address
192.168.1.73.When a successfully connection is established and authenticated with the sample login, typing characters in the ssh client should be echoed back from the ESP32 SSH server. There's no additional console output at the server.
When connecting via a program such as putty, an example dialog box like this is expected: