Skip to content

Add Argus library for ride height sensor #98

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.

Sign up for GitHub

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

Closed
dhairs wants to merge 2 commits into from
Closed

Add Argus library for ride height sensor #98

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cb4fb59
Add Argus library for ride height sensor
dhairs Nov 23, 2025
1321c3c
Add documentation for driver
dhairs Nov 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions MODULE.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,18 @@ use_repo(openocd, "openocd")
dfu = use_extension("//tools/dfu:dfu.bzl", "dfu")
use_repo(dfu, "dfu")

# AFBR-S50 API Library
git_repository = use_repo_rule("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")

git_repository(
name = "afbr_s50_lib",
remote = "https://github.com/Broadcom/AFBR-S50-API.git",
commit = "68b16693bedae0704b49edcfa2a79e82e5520ae3",
shallow_since = "2024-01-01",
build_file = "//drivers:afbr-s50/afbr-s50.BUILD",

@gemini-code-assist gemini-code-assist Bot Nov 23, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The filename afbr-s50.BUILD and its location are unconventional. Standard Bazel practice is to name build files BUILD or BUILD.bazel and place them in the directory that represents the package. I'd suggest renaming drivers/afbr-s50/afbr-s50.BUILD to drivers/afbr-s50/BUILD.bazel and updating this build_file attribute to //drivers/afbr-s50:BUILD.bazel. This will make the project structure more intuitive and align with Bazel conventions.

)
Comment on lines +70 to +79

@gemini-code-assist gemini-code-assist Bot Nov 23, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

For consistency with how other external dependencies like openocd and dfu are managed, consider moving this git_repository definition into a module extension (.bzl file) and using use_extension and use_repo here. This improves maintainability by centralizing dependency logic and keeping MODULE.bazel cleaner.



# Hermetic Rust Toolchain
# rust = use_extension("@rules_rust//rust:extensions.bzl", "rust")
# rust.toolchain(
Expand Down
16 changes: 16 additions & 0 deletions drivers/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,3 +195,19 @@ Then, in your main loop, add the following:
// you may want to replace the 0.01f with the amount of time (in seconds) between LED updates
led_rainbow(0.01f); // updates the LED colors
```

### Other Drivers

#### AFBR-S50 API

To use the AFBR-S50 API, include the following in your Bazel BUILD file's `firmware_project_g4` declaration:

```bazel
extra_deps = [
"@afbr_s50_lib//:afbr_s50_lib"
],
```

If you already have an `extra_deps` attribute, just add the above line to the list.

Then, in your code, use the API as specified by the official documentation.
18 changes: 18 additions & 0 deletions drivers/afbr-s50/afbr-s50.BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
load("@rules_cc//cc:defs.bzl", "cc_library")

cc_library(
name = "afbr_s50_lib",

srcs = glob(
["AFBR-S50/Lib/**/*.a"],
allow_empty = True
),

hdrs = glob(
["AFBR-S50/Include/**/*.h"],
allow_empty = True
),

includes = ["AFBR-S50/Include"],
visibility = ["//visibility:public"],
)
Comment on lines +3 to +18

@gemini-code-assist gemini-code-assist Bot Nov 23, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

critical

This cc_library definition has a critical issue and a few areas for improvement:

  1. [Critical] Incorrect srcs: The glob for srcs is too broad and will match all pre-compiled static libraries for every architecture (e.g., Cortex-M0, Cortex-M4F). This will cause linker errors due to duplicate symbol definitions. You should use select() to pick the correct library based on the target architecture.

  2. Unnecessary allow_empty: The hdrs glob probably shouldn't have allow_empty = True. If the headers are missing, the library is unusable, and the build should fail.

  3. Missing newline: The file is missing a newline at the end.

Here is a suggested replacement that addresses these points:

cc_library(
    name = "afbr_s50_lib",
    hdrs = glob(["AFBR-S50/Include/**/*.h"]),
    srcs = select({
        # TODO: Verify these platform constraints match your project's configuration.
        "@toolchains_arm_gnu//constraints:cortex_m4_fpu": glob(["AFBR-S50/Lib/GCC/ARM-Cortex-M4F/*.a"]),
        "@toolchains_arm_gnu//constraints:cortex_m0": glob(["AFBR-S50/Lib/GCC/ARM-Cortex-M0/*.a"]),
        "//conditions:default": [],
    }),
    includes = ["AFBR-S50/Include"],
    visibility = ["//visibility:public"],
)

Loading