br101 / zephyr-dw3000-decadriver

Zephyr Driver for Qorvo/Decawave DW3000
42 stars 19 forks source link

build errors against the new driver #19

Open mtfurlan opened 2 days ago

mtfurlan commented 2 days ago

I opened that PR to make a warning go away when this library isn't used, but now that I'm actually trying to compile against it I get linker errors

FAILED: zephyr/zephyr_pre0.elf zephyr/zephyr_pre0.map $workdir/build/zephyr/zephyr_pre0.map
: && ccache /opt/zephyr-sdk-0.16.8/arm-zephyr-eabi/bin/arm-zephyr-eabi-gcc  -gdwarf-4 zephyr/CMakeFiles/zephyr_pre0.dir/misc/empty_file.c.obj -o zephyr/zephyr_pre0.elf  zephyr/CMakeFiles/offsets.dir/./arch/arm/core/offsets/offsets.c.obj  -fuse-ld=bfd  -T  zephyr/linker_zephyr_pre0.cmd  -Wl,-Map=$workdir/build/zephyr && /usr/bin/cmake -E true
/opt/zephyr-sdk-0.16.8/arm-zephyr-eabi/bin/../lib/gcc/arm-zephyr-eabi/12.2.0/../../../../arm-zephyr-eabi/bin/ld.bfd: modules/zephyr-dw3000-decadriver/platform/lib..__modules__lib__zephyr-dw3000-decadriver__platform.a(deca_compat.c.obj): in function `dwt_check_dev_id':
$workdir/modules/lib/zephyr-dw3000-decadriver/platform/deca_compat.c:2341: undefined reference to `ull_check_dev_id'
/opt/zephyr-sdk-0.16.8/arm-zephyr-eabi/bin/../lib/gcc/arm-zephyr-eabi/12.2.0/../../../../arm-zephyr-eabi/bin/ld.bfd: modules/zephyr-dw3000-decadriver/platform/lib..__modules__lib__zephyr-dw3000-decadriver__platform.a(deca_compat.c.obj): in function `dwt_getframelength':
$workdir/modules/lib/zephyr-dw3000-decadriver/platform/deca_compat.c:2571: undefined reference to `ull_getframelength'
collect2: error: ld returned 1 exit status

I played around till I got it to build, you can see my work at mtfurlan/zephyr-dw3000-decadriver branch build/make-compile

changes:

I would really like if we could use the dwt_uwb_driver/CMakeLists.txt, but I can't make that actually work, it's not getting compile flags from zephyr I asked in zephyr discord about this, but I haven't had good luck getting questions answered there

Some questions for you:

  1. how are you making the master branch build?

  2. Should I PR what I have working, or should we wait to try to get including their cmakelists working?

  3. how much would you value having the samples in this repo with some CI to build them and some PR checks to ensure they build? I dunno if I can get $work to let me have the time, but it's a thing I know how to do

  4. did you modify the stuff in dwt_uwb_driver at all? What you have here doesn't quite match what I found in DW3_QM33_SDK_1.0.1.zip as of 2024-11-01, the difference at a glance seems to be the ull functions

  5. Would you accept a PR to dos2unix the line endings and maybe strip trailing whitespace? Git can automatically convert to carriage returns on windows, but it's not great going the other way

  6. Looking at DW3_QM33_SDK_1.0.1/Drivers/API/Build_Platforms/nRF52840-DK/Source/platform they have a different organization(deca_mutex.c, decasleep.c, etc), but it seems like the only differences are platform stuff handled by the `dw300*` functions As long as it compiles this is probably fine?

br101 commented 1 day ago

Hi, at first let me explain what I'm trying to achieve here in this repository. First task was to make the driver from Qorvo to compile under Zephyr. Second is to remove their IOCTL abstraction layer because it results in two huge functions to be included without the ability to optimize them out. It's a huge waste of space on already constrained platforms and a very bad design in my opinion.

So that's why i made changes to the driver files and added a new deca_compat.c and added the deca_ull.h. It's also the reason why we can't use their CMakeLists. I tried to make the changes to their code minimal because I expect new releases from Qorvo and would like to include them with minimal effort. That's also why I didn't convert or re-format their source files.

I just noticed that the master branch doesn't build properly (I actually use another repository https://github.com/br101/dw3000-decadriver-source in my daily work because I have to support other platforms as well) and I will try to fix it.

Building platform code in the same library makes sense, I will take that over.

I have another repository for the examples (https://github.com/br101/zephyr-dw3000-examples), but it's not updated.

So let's quickly go thru your questions:

  1. will have a look
  2. we can't use their CMakeLists for the reasons explained above
  3. i would prefer to update https://github.com/br101/zephyr-dw3000-example
  4. yes i did modify to remove the IOCTL (more work to be done)
  5. yes i would accept it, but it makes diffing against new releases a bit more hard
  6. yes as long as these functions are available it's fine
br101 commented 1 day ago
  1. it compiles and works here under NCS v2.7.0
mtfurlan commented 3 hours ago

Alright, that makes a lot more sense now, thanks

I still can't compile against your master branch with NCS 2.6.2 or mainline zephyr

I made a demonstrator, and figured out that it compiles if you're doing simple stuff (initialize it enough to rundwt_check_dev_id), but stuff like ex_06a_ss_twr_initiator and ex_06b_ss_twr_responder examples trips this issue

dwt_getframelength as defined in platform/deca_compat.c calls ull_getframelength, which is defined as static by dwt_uwb_driver/dw3000/dw3000_device.c, so the linker refuses (weirdly, it's not static in dwt_uwb_driver/dw3720/dw3720_device.c?)

Also, we should suppress the unused function warnings with

zephyr_library_cc_option(-Wno-unused-function)

or something, dwt_ioctl and dwt_dbg_fn aren't called and cause warnings that should go away