Skip to content
This repository has been archived by the owner on Nov 11, 2024. It is now read-only.

Improvements for newer Zephyr versions #170

Merged
merged 3 commits into from
Nov 21, 2023

Conversation

rexut
Copy link
Contributor

@rexut rexut commented Nov 17, 2023

This will respect changes to the Zephyr RTOS interfaces since v3.1

  1. since v3.1: All Zephyr public headers have been moved to include/zephyr, meaning they need to be prefixed with <zephyr/...> when included.
  2. since v3.4: SYS_INIT callback no longer requires a device argument. The new callback signature is int f(void).
  3. since v3.5: Random API header <zephyr/random/rand32.h> is deprecated in favor of <zephyr/random/random.h>. The old header will be removed in future releases and its usage should be avoided.

@RobMeades
Copy link
Contributor

Hi, and many thanks for this. Looks good, just going to push it through the test system to be quite sure.

@RobMeades
Copy link
Contributor

RobMeades commented Nov 19, 2023

Hmmm, are you sure about the exact Zephyr versions? Our test system uses Zephyr version 3.2.99, as comes with nrfConnect 2.3.0, and applying this PR with that version results in compilation errors, see attached.

@rexut
Copy link
Contributor Author

rexut commented Nov 20, 2023

Hmmm, are you sure about the exact Zephyr versions? Our test system uses Zephyr version 3.2.99, as comes with nrfConnect 2.3.0, and applying this PR with that version results in compilation errors, see attached.

The root of these side effects is the Nordic SDK. This comes with many patches to the original Zephyr project sources and its strategy disrupts the original release scheme. Note that Zephyr version 3.2.99 as "unstable" is in nrfConnect version 2.3.0 (declared as stable by Nordic). This is a mismatch.

However, the original Zephyr versions should be okay. All three comparisons based on the original release notes / migration guide:

  1. since Zephyr v3.1: https://docs.zephyrproject.org/latest/releases/release-notes-3.1.html

    "… All Zephyr public headers have been moved to include/zephyr, meaning they need to be prefixed with <zephyr/...> when included. …"

  2. since Zephyr v3.4: https://docs.zephyrproject.org/latest/releases/release-notes-3.4.html

    "… SYS_INIT callback no longer requires a device argument. The new callback signature is int f(void). …"

  3. since Zephyr v3.5: https://docs.zephyrproject.org/latest/releases/migration-guide-3.5.html

    "… Random API header <zephyr/random/rand32.h> is deprecated in favor of <zephyr/random/random.h>. …"

Maybe, that the comparitions have to extend to respect the nrfConnect version scheme too. Give me some time to find it out here locally.

@RobMeades
Copy link
Contributor

The root of these side effects is the Nordic SDK.

Maybe, that the comparitions have to extend to respect the nrfConnect version scheme too. Give me some time to find it out here locally.

Ugh, understood. I will start a discussion internally about whether we should go "native" Zephyr; the Nordic stuff is mostly driven by our short range modules (Wifi/BLE) (I'm more from the cellular side of our business).

@rexut
Copy link
Contributor Author

rexut commented Nov 20, 2023

The root of these side effects is the Nordic SDK.

Maybe, that the comparitions have to extend to respect the nrfConnect version scheme too. Give me some time to find it out here locally.

Ugh, understood. I will start a discussion internally about whether we should go "native" Zephyr; the Nordic stuff is mostly driven by our short range modules (Wifi/BLE) (I'm more from the cellular side of our business).

👍🏻

BUT: the PR has at least one deficit – it doesn't catch the new zephyr include prefix in zephyr/test/u_main.c. Error is:

.../workspace/ubxlib_temp_rmea@7/ubxlib/zephyr/test/u_main.c:29:10: fatal error: kernel.h: No such file or directory
   29 | #include "kernel.h"
      |          ^~~~~~~~~~

I'll fix this in same way as for port/platform/zephyr/app/u_main.c L45 – it's a low hanging fruit.

@rexut
Copy link
Contributor Author

rexut commented Nov 20, 2023

Hmmm, are you sure about the exact Zephyr versions? Our test system uses Zephyr version 3.2.99, as comes with nrfConnect 2.3.0, and applying this PR with that version results in compilation errors, see attached.

Okay, you are right. The PR is not quite right. I misinterpreted the kernel version template for the automatic generation of the central Zephyr C header <version.h> and did not use the correct version number for the comparison. Correct is the predefine KERNEL_VERSION_NUMBER, a 16-bit 24-bit number suitable for the comparison with the result from the macro ZEPHYR_VERSION(Mjr,Min,Pat).

I have adapted the PR again, rebased it and built and tested it with all Zephyr versions from 3.0 to 3.5 with a small example for a u-blox M10 module. Now it really works as expected. I haven't tested against an nRFConnect >= 2.3 yet, but it should work the same way.

@RobMeades
Copy link
Contributor

That's excellent news, thanks very much for persisting with this, I'll run it internally again.

Copy link
Contributor

@RobMeades RobMeades left a comment

Choose a reason for hiding this comment

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

Again, thanks SO much for this, really appreciate it.

A quick check suggests just one missed out:

/home/ubxlib/workspace/ubxlib_temp_rmea@5/ubxlib/port/test/u_port_test.c:79:11: fatal error: irq_offload.h: No such file or directory
   79 | # include <irq_offload.h> // To test semaphore from ISR in zephyr

You probably wouldn't have compiled with CONFIG_IRQ_OFFLOAD, it is only required when we are running Zephyr on posix. If you fix up that one then, fingers crossed, we're good to go; I don't anticipate any problems when running the tests.

@rexut
Copy link
Contributor Author

rexut commented Nov 21, 2023

You probably wouldn't have compiled with CONFIG_IRQ_OFFLOAD, it is only required when we are running Zephyr on posix. If you fix up that one then, fingers crossed, we're good to go; I don't anticipate any problems when running the tests.

Correct, I've never tried out any test on Zephyr POSIX platform with enabled IRQ offloads. Sorry for that, here it is again.

port/test/u_port_test.c Outdated Show resolved Hide resolved
All Zephyr public headers have been moved to `include/zephyr`, meaning
they need to be prefixed with `<zephyr/...>` when included.

Signed-off-by: Stephan Linz <[email protected]>
`SYS_INIT` callback no longer requires a `device` argument.
The new callback signature is `int f(void)`.

Signed-off-by: Stephan Linz <[email protected]>
Random API header `<zephyr/random/rand32.h>` is deprecated in
favor of `<zephyr/random/random.h>`. The old header will be
removed in future releases and its usage should be avoided.

Signed-off-by: Stephan Linz <[email protected]>
@RobMeades
Copy link
Contributor

Cool, compilation now good, tests are running...

Copy link
Contributor

@RobMeades RobMeades left a comment

Choose a reason for hiding this comment

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

Perfect: tests all pass, merging...

@RobMeades RobMeades merged commit 84d6cd9 into u-blox:master Nov 21, 2023
1 check passed
@rexut rexut deleted the contrib/zephyr/fixes branch November 21, 2023 15:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants