Skip to content

generate defmt::Format for enums #770

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.

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

Merged
merged 4 commits into from
Nov 23, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ jobs:
- { rust: stable, vendor: Spansion, options: "--atomics" }
- { rust: stable, vendor: STMicro, options: "" }
- { rust: stable, vendor: STMicro, options: "--atomics" }
- { rust: stable, vendor: STM32-patched, options: "--strict --pascal_enum_values --max_cluster_size --atomics --atomics_feature atomics" }
- { rust: stable, vendor: STM32-patched, options: "--strict --pascal_enum_values --max_cluster_size --atomics --atomics_feature atomics --impl_debug --impl-defmt defmt" }
- { rust: stable, vendor: Toshiba, options: all }
- { rust: stable, vendor: Toshiba, options: "" }
# Test MSRV
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/).
- *breaking change* Always numerates field arrays from 0
- Support of default value for `EnumeratedValues`
- move `Config` to `config` module
- add `impl-defmt` config flag

## [v0.30.3] - 2023-11-19

Expand Down
3 changes: 3 additions & 0 deletions ci/script.sh
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ main() {
if [[ "$options" == *"--atomics"* ]]; then
echo 'portable-atomic = { version = "1.4", default-features = false }' >> $td/Cargo.toml
fi
if [[ "$options" == *"--impl-defmt"* ]]; then
echo 'defmt = { version = "0.3.5", optional = true }' >> $td/Cargo.toml
fi
echo '[profile.dev]' >> $td/Cargo.toml
echo 'incremental = false' >> $td/Cargo.toml

Expand Down
1 change: 1 addition & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub struct Config {
pub max_cluster_size: bool,
pub impl_debug: bool,
pub impl_debug_feature: Option<String>,
pub impl_defmt: Option<String>,
pub output_dir: Option<PathBuf>,
pub input: Option<PathBuf>,
pub source_type: SourceType,
Expand Down
21 changes: 7 additions & 14 deletions src/generate/peripheral.rs
Original file line number Diff line number Diff line change
Expand Up @@ -622,34 +622,27 @@ fn register_or_cluster_block(
});
}
}

let mut derive_debug = TokenStream::new();
if config.impl_debug {
let derive_debug = config.impl_debug.then(|| {
if let Some(feature_name) = &config.impl_debug_feature {
derive_debug.extend(quote! {
#[cfg_attr(feature = #feature_name, derive(Debug))]
});
quote!(#[cfg_attr(feature = #feature_name, derive(Debug))])
} else {
derive_debug.extend(quote! {
#[derive(Debug)]
});
quote!(#[derive(Debug)])
}
}
});

let name = if let Some(name) = name {
name.to_constant_case_ident(span)
} else {
Ident::new("RegisterBlock", span)
};

let accessors = if !accessors.is_empty() {
let accessors = (!accessors.is_empty()).then(|| {
quote! {
impl #name {
#accessors
}
}
} else {
quote! {}
};
});

Ok(quote! {
///Register block
Expand Down
79 changes: 43 additions & 36 deletions src/generate/register.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,11 @@ pub fn render_register_mod(
let open = Punct::new('{', Spacing::Alone);
let close = Punct::new('}', Spacing::Alone);

let debug_feature = config
.impl_debug_feature
.as_ref()
.map(|feature| quote!(#[cfg(feature=#feature)]));

if let Some(cur_fields) = register.fields.as_ref() {
// filter out all reserved fields, as we should not generate code for
// them
Expand Down Expand Up @@ -284,12 +289,8 @@ pub fn render_register_mod(
)?;
}
} else if !access.can_read() || register.read_action.is_some() {
if let Some(feature) = &config.impl_debug_feature {
r_debug_impl.extend(quote! {
#[cfg(feature=#feature)]
});
}
r_debug_impl.extend(quote! {
#debug_feature
impl core::fmt::Debug for crate::generic::Reg<#regspec_ident> {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
write!(f, "(not readable)")
Expand All @@ -298,27 +299,17 @@ pub fn render_register_mod(
});
} else {
// no register fields are defined so implement Debug to get entire register value
if let Some(feature) = &config.impl_debug_feature {
r_debug_impl.extend(quote! {
#[cfg(feature=#feature)]
});
}
r_debug_impl.extend(quote! {
#debug_feature
impl core::fmt::Debug for R {
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
write!(f, "{}", self.bits())
}
}
});
if let Some(feature) = &config.impl_debug_feature {
r_debug_impl.extend(quote! {
#[cfg(feature=#feature)]
});
}
r_debug_impl.extend(quote! {
#debug_feature
impl core::fmt::Debug for crate::generic::Reg<#regspec_ident> {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
self.read().fmt(f)
core::fmt::Debug::fmt(&self.read(), f)
}
}
});
Expand Down Expand Up @@ -450,15 +441,15 @@ fn render_register_mod_debug(
let open = Punct::new('{', Spacing::Alone);
let close = Punct::new('}', Spacing::Alone);
let mut r_debug_impl = TokenStream::new();
let debug_feature = config
.impl_debug_feature
.as_ref()
.map(|feature| quote!(#[cfg(feature=#feature)]));

// implement Debug for register readable fields that have no read side effects
if access.can_read() && register.read_action.is_none() {
if let Some(feature) = &config.impl_debug_feature {
r_debug_impl.extend(quote! {
#[cfg(feature=#feature)]
});
}
r_debug_impl.extend(quote! {
#debug_feature
impl core::fmt::Debug for R #open
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result #open
f.debug_struct(#name)
Expand Down Expand Up @@ -496,25 +487,17 @@ fn render_register_mod_debug(
#close
#close
});
if let Some(feature) = &config.impl_debug_feature {
r_debug_impl.extend(quote! {
#[cfg(feature=#feature)]
});
}
r_debug_impl.extend(quote! {
#debug_feature
impl core::fmt::Debug for crate::generic::Reg<#regspec_ident> {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
self.read().fmt(f)
core::fmt::Debug::fmt(&self.read(), f)
}
}
});
} else if !access.can_read() || register.read_action.is_some() {
if let Some(feature) = &config.impl_debug_feature {
r_debug_impl.extend(quote! {
#[cfg(feature=#feature)]
});
}
r_debug_impl.extend(quote! {
#debug_feature
impl core::fmt::Debug for crate::generic::Reg<#regspec_ident> {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
write!(f, "(not readable)")
Expand Down Expand Up @@ -755,7 +738,7 @@ pub fn fields(
// else, generate enumeratedValues into a Rust enum with functions for each variant.
if variants.is_empty() {
// generate struct VALUE_READ_TY_A(fty) and From<fty> for VALUE_READ_TY_A.
add_with_no_variants(mod_items, &value_read_ty, &fty, &description, rv);
add_with_no_variants(mod_items, &value_read_ty, &fty, &description, rv, config);
} else {
// do we have finite definition of this enumeration in svd? If not, the later code would
// return an Option when the value read from field does not match any defined values.
Expand All @@ -770,6 +753,7 @@ pub fn fields(
&fty,
&description,
rv,
config,
);
has_reserved_variant = false;
} else {
Expand All @@ -780,6 +764,7 @@ pub fn fields(
&fty,
&description,
rv,
config,
);
has_reserved_variant = evs.values.len() != (1 << width);
}
Expand Down Expand Up @@ -1047,7 +1032,14 @@ pub fn fields(
// generate write value structure and From conversation if we can't reuse read value structure.
if writer_reader_different_enum {
if variants.is_empty() {
add_with_no_variants(mod_items, &value_write_ty, &fty, &description, rv);
add_with_no_variants(
mod_items,
&value_write_ty,
&fty,
&description,
rv,
config,
);
} else {
add_from_variants(
mod_items,
Expand All @@ -1056,6 +1048,7 @@ pub fn fields(
&fty,
&description,
rv,
config,
);
}
}
Expand Down Expand Up @@ -1324,7 +1317,13 @@ fn add_with_no_variants(
fty: &Ident,
desc: &str,
reset_value: Option<u64>,
config: &Config,
) {
let defmt = config
.impl_defmt
.as_ref()
.map(|feature| quote!(#[cfg_attr(feature = #feature, derive(defmt::Format))]));

let cast = if fty == "bool" {
quote! { val.0 as u8 != 0 }
} else {
Expand All @@ -1339,6 +1338,7 @@ fn add_with_no_variants(

mod_items.extend(quote! {
#[doc = #desc]
#defmt
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub struct #pc(#fty);
impl From<#pc> for #fty {
Expand All @@ -1364,7 +1364,13 @@ fn add_from_variants<'a>(
fty: &Ident,
desc: &str,
reset_value: Option<u64>,
config: &Config,
) {
let defmt = config
.impl_defmt
.as_ref()
.map(|feature| quote!(#[cfg_attr(feature = #feature, derive(defmt::Format))]));

let (repr, cast) = if fty == "bool" {
(quote! {}, quote! { variant as u8 != 0 })
} else {
Expand Down Expand Up @@ -1392,6 +1398,7 @@ fn add_from_variants<'a>(

mod_items.extend(quote! {
#[doc = #desc]
#defmt
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
#repr
pub enum #pc {
Expand Down
9 changes: 8 additions & 1 deletion src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,14 @@ fn run() -> Result<()> {
.arg(
Arg::new("impl_debug_feature")
.long("impl_debug_feature")
.help("add feature gating for block and register debug implementation")
.help("Add feature gating for block and register debug implementation")
.action(ArgAction::Set)
.value_name("FEATURE"),
)
.arg(
Arg::new("impl_defmt")
.long("impl-defmt")
Copy link
Member

Choose a reason for hiding this comment

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

this is not consistent with the other flags, unless we also bring in #771

Copy link
Member Author

Choose a reason for hiding this comment

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

do you suggest to support both - and _ for all flags, or only for old?

Copy link
Member

Choose a reason for hiding this comment

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

If we go with #771, I think it makes sense to only alias snake case for old flags.
Thus, if we do include #771 then an alias on impl-defmt isn't needed, but maybe it's nice to include for familiarity.

Copy link
Member Author

Choose a reason for hiding this comment

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

added alias in last commit

.help("Add automatic defmt implementation for enumerated values")
.action(ArgAction::Set)
.value_name("FEATURE"),
)
Expand Down