u-blox / ubxlib

Portable C libraries which provide APIs to build applications with u-blox products and services. Delivered as add-on to existing microcontroller and RTOS SDKs.
Apache License 2.0
287 stars 82 forks source link

Improvements for newer Zephyr versions #170

Closed rexut closed 7 months ago

rexut commented 7 months ago

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 commented 7 months ago

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

RobMeades commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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

rexut commented 7 months ago

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.

RobMeades commented 7 months ago

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