ragarnoy/a121-rs

Acconeer's A121 Rust Bindings

a121-rs's Issues

ugly math in main.rs

I haven't quite figured out the solution on arm yet, but it seems that adding this to the .cargo/config.toml

"-C", "link-arg=-lm"

combined with:

RUSTFLAGS='-L /usr/lib/arm-none-eabi/lib' cargo build --release

allows you to compile without needing any of them.

Use stubbed libraries for CI

Currently, the fetches the acconeer libraries and builds against them, but it would be way more interesting to generate stub libraries with the same symbols. The CI only needs to build against the library, not to flash a board, so in theory it'd work.

Add missing doc

Most of it can be taken from the C code and adapted to Rust

multiple definition of malloc, calloc, and free

Cross-posting this issue: esp-rs/esp-wifi#438

I use another crate which wraps some C code. That crate and this crate both rely on the C functions listed in the title. When both crates are used, the compiler dies of an aneurysm. Would it make sense to make a default-enabled feature which enables these functions inside the crate, but that users can opt-out of if they have implemented them inside their own parent project?

Implement assembly tests

The C SDK has assembly tests that could be ran by rust code

Handle multiple sensors

It -should- not be very hard since the difference is mainly that the CS line needs to be switched which is already done by the SpiDevice... But it hasn't been tried yet

Put defmt behind a feature

Currently defmt is forced on the end user, this needs to change

Before we go open-source to-do list!

• Review licenses we want for the repo
• fix ci to use secrets and pull blobs from external repo
• remove leftover target directories
• remove acconeer blobs from repo and the repo history

Spi mutex

I was going through the code and found this:

a121-rs/src/hal.rs

Lines 16 to 29 in 750ebff

	/// Global instance of a Mutex, wrapping a RefCell that optionally contains a mutable reference to a `SpiBus`.
	///
	/// `SPI_INSTANCE` is used to store and provide controlled access to the SPI device required by the radar sensor.
	/// The `Mutex` ensures thread-safe access in environments where multi-threading is possible, while the `RefCell`
	/// allows for mutable access to the SPI device. This setup is crucial for enabling SPI communications in a safe
	/// and controlled manner within the radar sensor's hardware abstraction layer.
	///
	/// # Safety
	///
	/// The access to the `SPI_INSTANCE` is controlled via a mutex to prevent concurrent access issues.
	/// However, care must be taken to ensure that the SPI device is properly initialized before use
	/// and is not accessed after it has been freed or gone out of scope.
	static SPI_INSTANCE: Mutex<CriticalSectionRawMutex, RefCell<Option<RefRadarSpi>>> =
	Mutex::new(RefCell::new(None));

A better API would be to let the user pass a `'static' instance of the hal impl themselves. This is already kinda done with the spi here:

a121-rs/src/radar.rs

Lines 64 to 69 in 750ebff

	pub fn new<SPI>(id: u32, spi: &'static mut SPI, interrupt: SINT) -> Self
	where
	SPI: SpiDevice<u8, Error = SpiErrorKind> + Send + 'static,
	{
	let hal = AccHalImpl::new(spi);
	hal.register();

This would also allow for not using the dyn trait and use full generics which would make things more efficient.

Adding support for the esp32c6

I'm trying to get this crate running on the esp32c6 (riscv32imac), which is very similar to the esp32c3 (riscv32imc).

The sample and static libraries provided by Acconeer (acconeer_esp32c3_xe121_a121-v1_5_0) already run fine on both the esp32c3 and the esp32c6, so I don't see any issues getting this crate to compile with the static binaries from there. Note that the esp32c3 sample/libraries are not available from the developer site directly (yet), but need to be requested via a support ticket.

So, I have already started working on this by creating a new no_std sample for the c6, see my fork here. I have placed the static libraries in ./acc inside the sample.

However, I'm still quite new to bindgen. In the current state, I'm running into two issues:

1. bingen seems to want to use riscv32-unknown-elf-gcc instead of riscv32-esp-elf-gcc (what espup provides and what acconeer used to compile the static binaries) -> any idea how to make it use riscv32-esp-elf-gcc?
2. After installing riscv32-unknown-elf-gcc via https://github.com/riscv-collab/riscv-gnu-toolchain, the compiler makes a little bit more progress before failing with:

   Compiling a121-rs v0.1.2 (/home/leon/Desktop/ivy/code/a121-rs)
The following warnings were emitted during compilation:

warning: [email protected]: Compiler version doesn't include clang or GCC: "riscv32-unknown-elf-gcc" "--version"

error: failed to run custom build command for `a121-rs v0.1.2 (/home/leon/Desktop/ivy/code/a121-rs)`

Caused by:
  process didn't exit successfully: `/home/leon/Desktop/ivy/code/a121-rs/examples/esp32c6-hal/target/debug/build/a121-rs-aaaf6a900988df96/build-script-build` (exit status: 101)
  --- stdout
  cargo:rustc-link-search=/home/leon/Desktop/ivy/code/a121-rs/lib
  cargo:rustc-link-lib=static=acconeer_a121
  cargo:rustc-link-lib=static=acc_detector_distance_a121
  cargo:rerun-if-changed=/home/leon/Desktop/ivy/code/a121-rs/rss/include
  TARGET = Some("riscv32imac-unknown-none-elf")
  OPT_LEVEL = Some("s")
  HOST = Some("x86_64-unknown-linux-gnu")
  cargo:rerun-if-env-changed=CC_riscv32imac-unknown-none-elf
  CC_riscv32imac-unknown-none-elf = None
  cargo:rerun-if-env-changed=CC_riscv32imac_unknown_none_elf
  CC_riscv32imac_unknown_none_elf = None
  cargo:rerun-if-env-changed=TARGET_CC
  TARGET_CC = None
  cargo:rerun-if-env-changed=CC
  CC = None
  RUSTC_LINKER = None
  cargo:rerun-if-env-changed=CROSS_COMPILE
  CROSS_COMPILE = None
  cargo:rerun-if-env-changed=CC_ENABLE_DEBUG_OUTPUT
  cargo:warning=Compiler version doesn't include clang or GCC: "riscv32-unknown-elf-gcc" "--version"
  cargo:rerun-if-env-changed=CRATE_CC_NO_DEFAULTS
  CRATE_CC_NO_DEFAULTS = None
  DEBUG = Some("true")
  CARGO_CFG_TARGET_FEATURE = Some("a,c,m")
  cargo:rerun-if-env-changed=CFLAGS_riscv32imac-unknown-none-elf
  CFLAGS_riscv32imac-unknown-none-elf = None
  cargo:rerun-if-env-changed=CFLAGS_riscv32imac_unknown_none_elf
  CFLAGS_riscv32imac_unknown_none_elf = None
  cargo:rerun-if-env-changed=TARGET_CFLAGS
  TARGET_CFLAGS = None
  cargo:rerun-if-env-changed=CFLAGS
  CFLAGS = None
  cargo:rerun-if-env-changed=AR_riscv32imac-unknown-none-elf
  AR_riscv32imac-unknown-none-elf = None
  cargo:rerun-if-env-changed=AR_riscv32imac_unknown_none_elf
  AR_riscv32imac_unknown_none_elf = None
  cargo:rerun-if-env-changed=TARGET_AR
  TARGET_AR = None
  cargo:rerun-if-env-changed=AR
  AR = None
  cargo:rerun-if-env-changed=ARFLAGS_riscv32imac-unknown-none-elf
  ARFLAGS_riscv32imac-unknown-none-elf = None
  cargo:rerun-if-env-changed=ARFLAGS_riscv32imac_unknown_none_elf
  ARFLAGS_riscv32imac_unknown_none_elf = None
  cargo:rerun-if-env-changed=TARGET_ARFLAGS
  TARGET_ARFLAGS = None
  cargo:rerun-if-env-changed=ARFLAGS
  ARFLAGS = None
  cargo:rustc-link-lib=static=log
  cargo:rustc-link-search=native=/home/leon/Desktop/ivy/code/a121-rs/examples/esp32c6-hal/target/riscv32imac-unknown-none-elf/debug/build/a121-rs-392adf043e17b2bb/out
  cargo:rerun-if-changed=c_src/wrapper.c
  cargo:rustc-link-lib=static=log

  --- stderr
  ACC_RSS_LIBS: /home/leon/Desktop/ivy/code/a121-rs/lib
  /usr/lib/llvm-14/lib/clang/14.0.0/include/inttypes.h:21:15: fatal error: 'inttypes.h' file not found
  thread 'main' panicked at /home/leon/Desktop/ivy/code/a121-rs/build.rs:60:40:
  Unable to generate bindings: ClangDiagnostic("/usr/lib/llvm-14/lib/clang/14.0.0/include/inttypes.h:21:15: fatal error: 'inttypes.h' file not found\n")
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Any idea how to continue from here?

Distance module is not behaving correctly

From my experience, I've had really odd results with the distance module

I either would get:

• No distances
• Correct distances that would never update if I blocked the radar with my hand
• Distance detector constantly recalibrating

Settle on size for calibration/preparation buffers

The calibrate and prepare functions both take in a buffer that is only useful for the duration of the calibration/preparation, their size can depend on the user's requirements, but we can assume it should be superior to the size returned by acc_rss_get_buffer_size

Refactor Sensor module

Current Sensor implementation is lackluster, the typestate doesn't work properly for a struct that is part of another struct, the typestate should only exist for Radar, radar should have the main big functions (calibration, prepare, measure, hibernate, etc), while sensor should just be an interface to the low level functions.

Split crate in two and have a separate a121-sys crate

The build chain is becoming too complex, creating a -sys crate would help

Rework RSS logging interface

There needs to be an alternative to the current way interfacing is done that preferably doesn't necessitate nightly

Update CI to run for examples

Currently the CI only runs for the A121 Library, it needs to have a new job that tries to compile the examples

Implement presence detector module

The distance detector module was implement, but the presence detector still needs to be done.

I have not looked at all at the code for now, no idea how hard it'd be.

Standard header files in tree

The include folder contains:

• stdio.h
• inttypes.h
• stdlib.h

These are standard headers that should be included with your compiler install.
Having them in tree manually will at some point cause trouble like it did with me when I compiled things with the esp compiler that uses different versions of those headers.

Don't create global alloc in libraries

I found this code:

a121-rs/src/lib.rs

Lines 18 to 26 in 750ebff

	static mut ARENA: [u8; 10000] = [0; 10000];
	#[global_allocator]
	static ALLOCATOR: Talck<spin::Mutex<()>, ClaimOnOom> = Talc::new(unsafe {
	// if we're in a hosted environment, the Rust runtime may allocate before
	// main() is called, so we need to initialize the arena automatically
	ClaimOnOom::new(Span::from_const_array(core::ptr::addr_of!(ARENA)))
	})
	.lock();

It should be up to the users to define the global allocator. They might want it in a special memory region or want to use a specific implementation.

But also, if all libraries do this, then they cannot coexist.

Remove static libs from repo and keep CI working

We need to figure out a decent way to host the libs somewhere and fetch them in the CI