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

Siwx917 Wifi improvements #44

Merged
merged 3 commits into from
Sep 25, 2024

Conversation

jerome-pouiller
Copy link
Contributor

  • Wifi was broken because of scheduler/clock issue
  • Fix support for Ipv6
  • Fix CI

Comment on lines 46 to +47
west twister --test sample.basic.helloworld -p siwx917_rb4338a -v --inline-logs $EXTRA_TWISTER_FLAGS
west twister --test sample.net.wifi -p siwx917_rb4338a -v --inline-logs $EXTRA_TWISTER_FLAGS
west twister --test sample.net.wifi -p siwx917_rb4338a -v --inline-logs -K $EXTRA_TWISTER_FLAGS
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just as a note for the future, twister supports multiple --test arguments, so we should likely just merge these into a single west twister --test ... --test ... command. I've postponed doing it since we have several PRs open which would conflict with such a change.

Another improvement would be to take advantage of GitHub workflow "matrix", so that GitHub would run in parallel the same workflow with different input parameters for each instance. The parameter could e.g. the a specific platform to build against, i.e. this becomes more relevant as we grow the list of platforms covered by our CI.

https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/running-variations-of-jobs-in-a-workflow

@jerome-pouiller
Copy link
Contributor Author

v2:

  • update hal_silabs revision

# WiseConnect create threads with realtime priority. Default (10kHz) clock tick
# prevent proper use of the system with these threads.
config SYS_CLOCK_TICKS_PER_SEC
default 1024
Copy link
Contributor

Choose a reason for hiding this comment

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

I think 1000 would make more sense (most new WiseConnect examples are also using 1000).

Copy link
Contributor

Choose a reason for hiding this comment

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

That seems strange, considering a 32768 Hz clock divided by 32 would give 1024. Are they not running on a low frequency timer (we are not yet, but were planning to)?

*/
memcpy(out, in, in_len);
if (in->sa_family == AF_INET6) {
for (i = 0; i < 4; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Using ARRAY_SIZE would be nicer.

SiWx917 use byte swapped IPv6 addresses. Once fixed, we are properly
able to use UDP and TCP connection over IPv6.

Signed-off-by: Jérôme Pouiller <[email protected]>
WiseConnect create threads with realtime priority. Default (10kHz) clock
tick prevent proper use of the system with these threads.

Signed-off-by: Jérôme Pouiller <[email protected]>
siwx917_rb4338a is not listed in the field "platform_allow" of
samples/net/wifi/sample.yaml. So we have to pass -K to twister for
execution of this test.

Wifi capability was also missed from siwx917_rb4338a.yaml.

Signed-off-by: Jérôme Pouiller <[email protected]>
@jerome-pouiller
Copy link
Contributor Author

v3:

  • Use ARRAY_SIZE()

@jhedberg
Copy link
Collaborator

@jerome-pouiller @silabs-TiborL is this good to be merged now?

@jerome-pouiller
Copy link
Contributor Author

@jerome-pouiller @silabs-TiborL is this good to be merged now?

@silabs-TiborL,are you able to approve the PR?

@jhedberg jhedberg merged commit 5c639ba into SiliconLabsSoftware:main Sep 25, 2024
4 checks passed
@jerome-pouiller jerome-pouiller deleted the siwx917 branch September 25, 2024 15:11
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