Northeastern-Electric-Racing / Northeastern-Electric-Racing/firmware-rs

utilities: cangen: Generate wrapper structs that allow you to create frames with declarative syntax instead of builder pattern

Open
#13 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
0
Forks
2
Avg merge
2d 17h
Merged PRs (30d)
5

Description

Description

Right now the cangen crate exposes raw bitfield_struct structs, which are initialized using the builder pattern. This is usually fine, but there are some cases in which the builder pattern can be kind of hard to read compared to the traditional declarative struct pattern.

For example, here's a struct getting initialized with the builder pattern:
Image

...versus the same code, but with the declarative pattern:
Image

Depending on the person, the declarative pattern is a tiny bit easier to read because you can differentiate between the CAN point name and the value that is being used to initialize it. It is slightly easier to parse when you are looking at it quickly. Also, the declarative pattern ensures every field of the struct is explicitly initialized, while the builder pattern leaves fields at defaults if you don't call their respective builder methods.

So, it may be a good idea for cangen to generate those types automatically.

Implementation?

cangen could either keep the raw bitfield_struct structs private and only expose the declarative wrapper structs. Or, it could expose both for users to choose between each pattern as they wish (though this would make the type names kind of annoying).

Here is an example of how a generated wrapper struct could look:

pub struct AlphaCellDataDebug {
    pub therm: f32,
    pub voltage_a: f32,
    pub voltage_b: f32,
    pub chip_id: u8,
    pub cell_a: u8,
    pub cell_b: u8,
    pub discharging_a: bool,
    pub discharging_b: bool,
    pub cvs_a: bool,
    pub cvs_b: bool,
    pub ow_a: bool,
    pub ow_b: bool,
}
impl AlphaCellDataDebug {
    pub fn as_frame(&self) -> Frame {
        let frame = cangen::AlphaCellDataDebug::new()
        .with_therm(self.therm)
        .with_voltage_a(self.voltage_a)
        .with_voltage_b(self.voltage_b)
        .with_chip_id(self.chip_id)
        .with_cell_a(self.cell_a)
        .with_cell_b(self.cell_b)
        .with_discharging_a(self.discharging_a)
        .with_discharging_b(self.discharging_b)
        .with_cvs_a(self.cvs_a)
        .with_cvs_b(self.cvs_b)
        .with_ow_a(self.ow_a)
        .with_ow_b(self.ow_b);
        
        frame.to_can_frame()
    }
}

It would probably also be a good idea for the as_frame() wrapper method to use the .with_xxx_checked() methods instead of the .with_xxx() methods, since the .with_xxx() methods will panic if the provided value doesn't fit. The implementation would have to choose whether to just truncate/saturate values that don't fit inside the bits (like the C cangen IIRC), or have as_frame(&self) return an Option<Frame> to align with .with_xxx_checked(). Or, the implementation could still just panic if we are fine with that.

Also, the as_frame(&self) would probably need to be generic over embedded_can::Frame like .to_can_frame() already is.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in the cangen crate by tracing how raw bitfield_struct types are generated and how to_can_frame is exposed. Compare the existing builder methods, including the checked variants, with the proposed declarative wrapper and resolve whether wrappers, raw types, or both are public. Done means generated wrappers initialize every field and provide an as_frame method with defined overflow behavior and embedded_can::Frame support.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
embedded-iot
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.