flipperzero-rs / flipperzero

Rust on the Flipper Zero
MIT License
518 stars 34 forks source link

generate-bindings: Transform Doxygen comments to Rustdoc #45

Closed str4d closed 1 year ago

str4d commented 1 year ago

This should make documentation like this more readable.

str4d commented 1 year ago

I attempted to test this by regenerating the bindings locally, but following the instructions in docs/updating-sdk.md I get:

     Running `target/debug/generate-bindings ../../flipperzero-firmware/build/f7-firmware-D/sdk`
Generating bindings for SDK 000E0000
warning: optimization flag '-fsingle-precision-constant' is not supported [-Wignored-optimization-argument]
/path/to/flipperzero/tools/../../flipperzero-firmware/build/f7-firmware-D/sdk/../../../toolchain/x86_64-linux/arm-none-eabi/include/stdlib.h:16:10: fatal error: 'stddef.h' file not found
clang diag: warning: optimization flag '-fsingle-precision-constant' is not supported [-Wignored-optimization-argument]

clang diagnosed error: /path/to/flipperzero/tools/../../flipperzero-firmware/build/f7-firmware-D/sdk/../../../toolchain/x86_64-linux/arm-none-eabi/include/stdlib.h:16:10: fatal error: 'stddef.h' file not found

thread 'main' panicked at 'failed to generate bindings', src/bin/generate-bindings.rs:215:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
dcoles commented 1 year ago

I just tried running your changes on my machine, but the doxygen-rs crate paniced:

> cargo run --release ..\..\flipperzero-firmware\build\f7-firmware-D\sdk\
   Compiling flipperzero-tools v0.1.0 (C:\Users\dcoles\workspace\flipperzero-rs\tools)
    Finished release [optimized] target(s) in 1.75s
     Running `target\release\generate-bindings.exe ..\..\flipperzero-firmware\build\f7-firmware-D\sdk\`
Generating bindings for SDK 000E0000
clang diag: warning: optimization flag '-fsingle-precision-constant' is not supported [-Wignored-optimization-argument]
clang diag: C:/Users/dcoles/workspace/flipperzero-rs/tools/../../flipperzero-firmware/build/f7-firmware-D/sdk//f7_sdk/lib/libusb_stm32/inc/hid_usage_simulation.h:16:9: warning: '_USB_HID_USAGE_SIMUL_H_' is used as a header guard here, followed by #define of a different macro [-Wheader-guard]
thread 'main' panicked at 'index out of bounds: the len is 1 but the index is 1', C:\Users\dcoles\.cargo\registry\src\github.com-1ecc6299db9ec823\doxygen-rs-0.3.0\src\ast\mod.rs:187:39
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
error: process didn't exit successfully: `target\release\generate-bindings.exe ..\..\flipperzero-firmware\build\f7-firmware-D\sdk\` (exit code: 101)

It looks like it might have run into a @param with no description and assumes that there always will be one (or at least a whitespace separator).

clang diagnosed error: /path/to/flipperzero/tools/../../flipperzero-firmware/build/f7-firmware-D/sdk/../../../toolchain/x86_64-linux/arm-none-eabi/include/stdlib.h:16:10: fatal error: 'stddef.h' file not found

I wonder why it's choking on stddef.h? You might want to try fetching the official 0.77.1 SDK and matching toolchain.

It's also possible the the version of Clang used by bindgen and the GCC toolchain are incompatible (stddef.h is typically defined by the compiler) or there's some issue with mixing Windows/Unix paths. I'll try running it on my Linux box.

str4d commented 1 year ago

I just tried running your changes on my machine, but the doxygen-rs crate paniced:

> cargo run --release ..\..\flipperzero-firmware\build\f7-firmware-D\sdk\
   Compiling flipperzero-tools v0.1.0 (C:\Users\dcoles\workspace\flipperzero-rs\tools)
    Finished release [optimized] target(s) in 1.75s
     Running `target\release\generate-bindings.exe ..\..\flipperzero-firmware\build\f7-firmware-D\sdk\`
Generating bindings for SDK 000E0000
clang diag: warning: optimization flag '-fsingle-precision-constant' is not supported [-Wignored-optimization-argument]
clang diag: C:/Users/dcoles/workspace/flipperzero-rs/tools/../../flipperzero-firmware/build/f7-firmware-D/sdk//f7_sdk/lib/libusb_stm32/inc/hid_usage_simulation.h:16:9: warning: '_USB_HID_USAGE_SIMUL_H_' is used as a header guard here, followed by #define of a different macro [-Wheader-guard]
thread 'main' panicked at 'index out of bounds: the len is 1 but the index is 1', C:\Users\dcoles\.cargo\registry\src\github.com-1ecc6299db9ec823\doxygen-rs-0.3.0\src\ast\mod.rs:187:39
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
error: process didn't exit successfully: `target\release\generate-bindings.exe ..\..\flipperzero-firmware\build\f7-firmware-D\sdk\` (exit code: 101)

It looks like it might have run into a @param with no description and assumes that there always will be one (or at least a whitespace separator).

If I can solve the compiler issue below, then I'd be happy to make patches to doxygen-rs fixing issues this PR finds.

clang diagnosed error: /path/to/flipperzero/tools/../../flipperzero-firmware/build/f7-firmware-D/sdk/../../../toolchain/x86_64-linux/arm-none-eabi/include/stdlib.h:16:10: fatal error: 'stddef.h' file not found

I wonder why it's choking on stddef.h? You might want to try fetching the official 0.77.1 SDK and matching toolchain.

I followed the docs and ran ./fbt first in the flipper firmware repo, which successfully downloaded the toolchain and built the firmware. There must be some difference in how it is interacting with my system compilers vs how bindgen is configured.

dcoles commented 1 year ago

There must be some difference in how it is interacting with my system compilers vs how bindgen is configured.

So far I haven't been able to reproduce this on my Linux box (Ubuntu 20.04.2 LTS on x86_64). It looks like it's loading /usr/lib/llvm-10/lib/libclang-10.so.1 (libclang1-10/focal,now 1:10.0.0-4ubuntu1 amd64) at runtime.

I do want to work out why this is failing. As soon as this logic gets moved to build.rs, then I expect to see this and similar issues popping up.

str4d commented 1 year ago

The machine on which I'm having issues is Ubuntu 22.04.1 LTS on x86_64. These are the C compilers that I have installed (as seen inside the tools/ subdir of this repo):

$ gcc --version
gcc (Ubuntu 11.3.0-1ubuntu1~22.04) 11.3.0
$ arm-none-eabi-gcc --version
arm-none-eabi-gcc (15:10.3-2021.07-4) 10.3.1 20210621 (release)
$ clang --version
Command 'clang' not found, but can be installed with:
sudo apt install clang
str4d commented 1 year ago

After installing Clang with sudo apt install clang, it works:

     Running `target/debug/generate-bindings ../../flipperzero-firmware/build/f7-firmware-D/sdk`
Generating bindings for SDK 000E0000
clang diag: warning: optimization flag '-fsingle-precision-constant' is not supported [-Wignored-optimization-argument]
clang diag: /home/str4d/dev/flipper/flipperzero/tools/../../flipperzero-firmware/build/f7-firmware-D/sdk/f7_sdk/lib/libusb_stm32/inc/hid_usage_simulation.h:16:9: warning: '_USB_HID_USAGE_SIMUL_H_' is used as a header guard here, followed by #define of a different macro [-Wheader-guard]
thread 'main' panicked at 'index out of bounds: the len is 1 but the index is 1', /home/str4d/.cargo/registry/src/github.com-1ecc6299db9ec823/doxygen-rs-0.3.0/src/ast/mod.rs:187:39
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

So the problem is specifically that generate-bindings requires system Clang to be present, and doesn't work if only system GCC is available. This may be fine, but it should be documented in docs/updating-sdk.md if intended.

dcoles commented 1 year ago

So the problem is specifically that generate-bindings requires system Clang to be present, and doesn't work if only system GCC is available. This may be fine, but it should be documented in docs/updating-sdk.md if intended

That's great to hear! I wonder why bindgen doesn't complain more about this (I'll have to see what other crates using bindgen do).

str4d commented 1 year ago

https://github.com/Techie-Pi/doxygen-rs/pull/6 has been merged and released as doxygen-rs 0.3.1, so I've force-pushed this PR to use it. generate-bindings now works for me, so I've added a second commit that shows the effect of regenerating the bindings with doxygen-rs transforming comments. I think the next step is examining the comments, and using them to suggest parsing and/or rendering improvements to doxygen-rs.

dcoles commented 1 year ago

Thank you very much for the PR! I'll push a new version shortly.

There was a very minor change in the generated bindings, so I amended the second commit.

dcoles commented 1 year ago

Just published flipperzero-0.7.1.

The new docs should be up shortly.

dcoles commented 1 year ago

Something a bit unusual is going on. All the item documentation now has the text "Re-export bindings" infront.

I suspect I need to remove this line.