Skip to content
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

Add enums for LPC55 pin codegen #1732

Merged
merged 3 commits into from
Apr 15, 2024
Merged

Add enums for LPC55 pin codegen #1732

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
48ada9e
Add enums for codegen types
mkeeter Apr 5, 2024
669a1c4
use derive(Default)
mkeeter Apr 15, 2024
b5af996
Use serde::default instead of unwrap_or_default
mkeeter Apr 15, 2024
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
4 changes: 2 additions & 2 deletions app/lpc55xpresso/app-sprot.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,12 @@ interrupts = {"flexcomm5.irq" = "spi-irq"}
# Out = MOSI on, MISO off
out_cfg = [
{ pin = { port = 0, pin = 8 }, alt = 3 },
{ pin = { port = 0, pin = 9 }, alt = 0, mode = "PullDown" },
{ pin = { port = 0, pin = 9 }, alt = 0, mode = "pulldown" },
]
# In = MISO on, MOSI off
in_cfg = [
{ pin = { port = 0, pin = 9 }, alt = 3 },
{ pin = { port = 0, pin = 8 }, alt = 0, mode = "PullDown" },
{ pin = { port = 0, pin = 8 }, alt = 0, mode = "pulldown" },
]
pins = [
# SCK
Expand Down
4 changes: 2 additions & 2 deletions app/oxide-rot-1/app-dev.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,12 @@ interrupts = {"flexcomm5.irq" = "spi-irq"}
# Out = MOSI on, MISO off
out_cfg = [
{ pin = { port = 0, pin = 8 }, alt = 3 },
{ pin = { port = 0, pin = 9 }, alt = 0, mode = "PullDown" },
{ pin = { port = 0, pin = 9 }, alt = 0, mode = "pulldown" },
]
# In = MISO on, MOSI off
in_cfg = [
{ pin = { port = 0, pin = 9 }, alt = 3 },
{ pin = { port = 0, pin = 8 }, alt = 0, mode = "PullDown" },
{ pin = { port = 0, pin = 8 }, alt = 0, mode = "pulldown" },
]
pins = [
# SCK
Expand Down
4 changes: 2 additions & 2 deletions app/oxide-rot-1/app.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,12 @@ interrupts = {"flexcomm5.irq" = "spi-irq"}
# Out = MOSI on, MISO off
out_cfg = [
{ pin = { port = 0, pin = 8 }, alt = 3 },
{ pin = { port = 0, pin = 9 }, alt = 0, mode = "PullDown" },
{ pin = { port = 0, pin = 9 }, alt = 0, mode = "pulldown" },
]
# In = MISO on, MOSI off
in_cfg = [
{ pin = { port = 0, pin = 9 }, alt = 3 },
{ pin = { port = 0, pin = 8 }, alt = 0, mode = "PullDown" },
{ pin = { port = 0, pin = 8 }, alt = 0, mode = "pulldown" },
]
pins = [
# SCK
Expand Down
4 changes: 2 additions & 2 deletions app/rot-carrier/app.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,12 +151,12 @@ interrupts = {"flexcomm3.irq" = "spi-irq"}
# Out = MOSI on, MISO off
out_cfg = [
{ pin = { port = 0, pin = 3 }, alt = 1 },
{ pin = { port = 0, pin = 2 }, alt = 0, mode = "PullDown" },
{ pin = { port = 0, pin = 2 }, alt = 0, mode = "pulldown" },
]
# In = MISO on, MOSI off
in_cfg = [
{ pin = { port = 0, pin = 2 }, alt = 1 },
{ pin = { port = 0, pin = 3 }, alt = 0, mode = "PullDown" },
{ pin = { port = 0, pin = 3 }, alt = 0, mode = "pulldown" },
]
pins = [
# SCK
Expand Down
113 changes: 70 additions & 43 deletions build/lpc55pins/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use serde::Deserialize;
use std::io::{BufWriter, Write};

#[derive(Clone, Debug, Deserialize)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
#[serde(rename_all = "lowercase", deny_unknown_fields)]
struct Pin {
port: usize,
pin: usize,
Expand All @@ -36,53 +36,82 @@ impl ToTokens for Pin {
}

#[derive(Clone, Debug, Deserialize)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
#[serde(rename_all = "lowercase", deny_unknown_fields)]
pub struct PinConfig {
pin: Pin,
alt: usize,
mode: Option<String>,
slew: Option<String>,
invert: Option<String>,
digimode: Option<String>,
opendrain: Option<String>,
direction: Option<String>,
mode: Option<Mode>,
slew: Option<Slew>,
invert: Option<Invert>,
digimode: Option<Digimode>,
opendrain: Option<Opendrain>,
direction: Option<Direction>,
name: Option<String>,
}

#[derive(Copy, Clone, Debug, Deserialize)]
#[serde(rename_all = "lowercase")]
enum Mode {
NoPull,
PullDown,
PullUp,
Repeater,
}

#[derive(Copy, Clone, Debug, Deserialize)]
#[serde(rename_all = "lowercase")]
enum Slew {
Standard,
Fast,
}

#[derive(Copy, Clone, Debug, Deserialize)]
#[serde(rename_all = "lowercase")]
pub enum Invert {
Disable,
Enabled,
}

#[derive(Copy, Clone, Debug, Deserialize)]
#[serde(rename_all = "lowercase")]
pub enum Opendrain {
Normal,
Opendrain,
}

#[derive(Copy, Clone, Debug, Deserialize)]
#[serde(rename_all = "lowercase")]
pub enum Digimode {
Analog,
Digital,
}

#[derive(Copy, Clone, Debug, Deserialize)]
#[serde(rename_all = "lowercase")]
pub enum Direction {
Input,
Output,
}

impl PinConfig {
fn get_mode(&self) -> String {
match &self.mode {
None => "NoPull".to_string(),
Some(s) => s.to_string(),
}
fn get_mode(&self) -> Mode {
self.mode.unwrap_or(Mode::NoPull)
Copy link
Collaborator

Choose a reason for hiding this comment

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

On all of these, I recommend derive(Default) on the enum with a #[default] tag on the variant you want. These become unwrap_or_default. Or, at that point, you might leave the defaulting to the caller, depending on how it's used.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nice, that comes out slightly shorter

}

fn get_slew(&self) -> String {
match &self.slew {
None => "Standard".to_string(),
Some(s) => s.to_string(),
}
fn get_slew(&self) -> Slew {
self.slew.unwrap_or(Slew::Standard)
}

fn get_invert(&self) -> String {
match &self.invert {
None => "Disable".to_string(),
Some(s) => s.to_string(),
}
fn get_invert(&self) -> Invert {
self.invert.unwrap_or(Invert::Disable)
}

fn get_digimode(&self) -> String {
match &self.digimode {
None => "Digital".to_string(),
Some(s) => s.to_string(),
}
fn get_digimode(&self) -> Digimode {
self.digimode.unwrap_or(Digimode::Digital)
}

fn get_opendrain(&self) -> String {
match &self.opendrain {
None => "Normal".to_string(),
Some(s) => s.to_string(),
}
fn get_opendrain(&self) -> Opendrain {
self.opendrain.unwrap_or(Opendrain::Normal)
}

fn get_alt(&self) -> usize {
Expand All @@ -98,12 +127,13 @@ impl ToTokens for PinConfig {
fn to_tokens(&self, tokens: &mut TokenStream) {
let final_pin = self.pin.to_token_stream();
let alt_num = format_ident!("Alt{}", self.get_alt());
let mode = format_ident!("{}", self.get_mode());
let slew = format_ident!("{}", self.get_slew());
let invert = format_ident!("{}", self.get_invert());
let digimode = format_ident!("{}", self.get_digimode());
let od = format_ident!("{}", self.get_opendrain());

let mode = format_ident!("{}", format!("{:?}", self.get_mode()));
let slew = format_ident!("{}", format!("{:?}", self.get_slew()));
let invert = format_ident!("{}", format!("{:?}", self.get_invert()));
let digimode =
format_ident!("{}", format!("{:?}", self.get_digimode()));
let od = format_ident!("{}", format!("{:?}", self.get_opendrain()));
Copy link
Member

Choose a reason for hiding this comment

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

can't these just be

Suggested change
let mode = format_ident!("{}", format!("{:?}", self.get_mode()));
let slew = format_ident!("{}", format!("{:?}", self.get_slew()));
let invert = format_ident!("{}", format!("{:?}", self.get_invert()));
let digimode =
format_ident!("{}", format!("{:?}", self.get_digimode()));
let od = format_ident!("{}", format!("{:?}", self.get_opendrain()));
let mode = format_ident!("{:?}", self.get_mode());
let slew = format_ident!("{:?}", self.get_slew());
let invert = format_ident!("{:?}", format!("{:?}", self.get_invert());
let digimode =
format_ident!("{:?}", self.get_digimode());
let od = format_ident!("{:?}", self.get_opendrain());

AFAICT, we shouldn't need the second string allocation?

Copy link
Member

Choose a reason for hiding this comment

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

also, it occurs to me an alternative to using the Debug impl could be implementing ToTokens for the various enums, but...that would probably just internally use syn::Ident::new or something, which would end up looking similarish. But, you could hard-code the string, which is probably either better or worse...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately, the former suggestion doesn't work, spitting out an impressive 4 errors per line:

error[E0277]: the trait bound `Mode: IdentFragment` is not satisfied
   --> build/lpc55pins/src/lib.rs:131:20
    |
131 |         let mode = format_ident!("{:?}", self.get_mode());
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |                    |
    |                    the trait `IdentFragment` is not implemented for `Mode`
    |                    required by a bound introduced by this call
    |
    = help: the following other types implement trait `IdentFragment`:
              &T
              &mut T
              Cow<'_, T>
              bool
              char
              proc_macro2::Ident
              std::string::String
              str
            and 6 others
    = note: required for `&Mode` to implement `IdentFragment`
note: required by a bound in `IdentFragmentAdapter`
   --> /Users/mjk/.cargo/registry/src/github.com-1ecc6299db9ec823/quote-1.0.35/src/runtime.rs:494:36
    |
494 | pub struct IdentFragmentAdapter<T: IdentFragment>(pub T);
    |                                    ^^^^^^^^^^^^^ required by this bound in `IdentFragmentAdapter`
    = note: this error originates in the macro `$crate::format_ident_impl` which comes from the expansion of the macro `format_ident` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `Mode: IdentFragment` is not satisfied
   --> build/lpc55pins/src/lib.rs:131:20
    |
131 |         let mode = format_ident!("{:?}", self.get_mode());
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `IdentFragment` is not implemented for `Mode`
    |
    = help: the following other types implement trait `IdentFragment`:
              &T
              &mut T
              Cow<'_, T>
              bool
              char
              proc_macro2::Ident
              std::string::String
              str
            and 6 others
    = note: required for `&Mode` to implement `IdentFragment`
note: required by a bound in `IdentFragmentAdapter`
   --> /Users/mjk/.cargo/registry/src/github.com-1ecc6299db9ec823/quote-1.0.35/src/runtime.rs:494:36
    |
494 | pub struct IdentFragmentAdapter<T: IdentFragment>(pub T);
    |                                    ^^^^^^^^^^^^^ required by this bound in `IdentFragmentAdapter`
    = note: this error originates in the macro `$crate::format_ident_impl` which comes from the expansion of the macro `format_ident` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: `IdentFragmentAdapter<&Mode>` doesn't implement `Debug`
   --> build/lpc55pins/src/lib.rs:131:20
    |
131 |         let mode = format_ident!("{:?}", self.get_mode());
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `IdentFragmentAdapter<&Mode>` cannot be formatted using `{:?}` because it doesn't implement `Debug`
    |
    = help: the trait `Debug` is not implemented for `IdentFragmentAdapter<&Mode>`
    = note: this error originates in the macro `$crate::__export::format_args` which comes from the expansion of the macro `format_ident` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `Mode: IdentFragment` is not satisfied
   --> build/lpc55pins/src/lib.rs:131:20
    |
131 |         let mode = format_ident!("{:?}", self.get_mode());
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `IdentFragment` is not implemented for `Mode`
    |
    = help: the following other types implement trait `IdentFragment`:
              &T
              &mut T
              Cow<'_, T>
              bool
              char
              proc_macro2::Ident
              std::string::String
              str
            and 6 others
    = note: required for `&Mode` to implement `IdentFragment`
note: required by a bound in `IdentFragmentAdapter`
   --> /Users/mjk/.cargo/registry/src/github.com-1ecc6299db9ec823/quote-1.0.35/src/runtime.rs:494:36
    |
494 | pub struct IdentFragmentAdapter<T: IdentFragment>(pub T);
    |                                    ^^^^^^^^^^^^^ required by this bound in `IdentFragmentAdapter`
    = note: this error originates in the macro `$crate::__export::format_args` which comes from the expansion of the macro `format_ident` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0599]: the method `span` exists for struct `IdentFragmentAdapter<&Mode>`, but its trait bounds were not satisfied
   --> build/lpc55pins/src/lib.rs:131:20
    |
131 |         let mode = format_ident!("{:?}", self.get_mode());
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ method cannot be called on `IdentFragmentAdapter<&Mode>` due to unsatisfied trait bounds
    |
    = note: the following trait bounds were not satisfied:
            `&Mode: IdentFragment`
    = note: this error originates in the macro `$crate::format_ident_impl` which comes from the expansion of the macro `format_ident` (in Nightly builds, run with -Z macro-backtrace for more info)

(an easy way to test is running cargo xtask build --dirty app/oxide-rot-1/app.toml swd)

I experimented with implementing some of the syn traits, but am iffy about the amount of boilerplate required...

Copy link
Member

Choose a reason for hiding this comment

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

Oh, huh, I was apparently wrong about how format_ident! works!

I suppose another option would just be to write

syn::Ident::new(format!("{:?}", self.get_mode()), proc_macro2::Span::call_site())

to avoid the second allocation, although this is, admittedly, uglier...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I'm on the fence here; if all solutions are equally ugly, then might as well pick the shorter one.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah...the impact on build time is probably not meaningful. Carry on.

tokens.append_all(final_pin);
tokens.append_all(quote::quote! {
AltFn::#alt_num,
Expand Down Expand Up @@ -135,16 +165,13 @@ pub fn codegen(pins: Vec<PinConfig>) -> Result<()> {
writeln!(&mut file, "iocon.iocon_configure(")?;
writeln!(&mut file, "{}", p.to_token_stream())?;
writeln!(&mut file, ");")?;

match p.direction {
None => (),
Some(d) => {
writeln!(&mut file, "iocon.set_dir(")?;
writeln!(&mut file, "{}", p.pin.to_token_stream())?;
if d == "output" {
writeln!(&mut file, "Direction::Output")?;
} else {
writeln!(&mut file, "Direction::Input")?;
}
writeln!(&mut file, "Direction::{d:?}")?;
writeln!(&mut file, ");")?;
}
}
Expand Down
Loading