Skip to content

Comments

fix: stm32h7rs usb_refck panic message now compiles#3927

Closed
friedman-ionq wants to merge 3 commits intoembassy-rs:mainfrom
friedman-ionq:fix_stm32h7rs_building
Closed

fix: stm32h7rs usb_refck panic message now compiles#3927
friedman-ionq wants to merge 3 commits intoembassy-rs:mainfrom
friedman-ionq:fix_stm32h7rs_building

Conversation

@friedman-ionq
Copy link
Contributor

With the following in Cargo.toml:

embassy_stm32 = {"0.2", features = [   "memory-x",
    "stm32h7s3l8",
    "defmt",
    "time-driver-any",
    "exti",
    "unstable-pac",
] }

the crate would not compile. This fixes the issue.

@Dirbaio
Copy link
Member

Dirbaio commented Feb 27, 2025

Could you add this feature combination to ci.sh so it'd have been caught? thanks!

@friedman-ionq
Copy link
Contributor Author

friedman-ionq commented Feb 27, 2025

Could you add this feature combination to ci.sh so it'd have been caught? thanks!

Sure. Turns out the minimum repeatable feature is just "stm32h7s3l8". Which is surprising, because there already is a test case. Digging deeper.

@Dirbaio
Copy link
Member

Dirbaio commented Feb 27, 2025

maybe it's due to defmt. When it's enabled the panic! macro is overridden, so maybe it works with and fails without.

@friedman-ionq
Copy link
Contributor Author

maybe it's due to defmt. When it's enabled the panic! macro is overridden, so maybe it works with and fails without.

$ ./ci.sh
thread 'main' panicked at src/bin/cargo-batch.rs:136:85:
called `Result::unwrap()` on an `Err` value: the package 'embassy-stm32-tests' does not contain this feature: defmt
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@Dirbaio
Copy link
Member

Dirbaio commented Feb 27, 2025

the embassy-stm32-tests crate doesn't have it, embassy-stm32 does.

@friedman-ionq
Copy link
Contributor Author

the embassy-stm32-tests crate doesn't have it, embassy-stm32 does.

``~/work/embassy (main)$ git diff
diff --git a/ci.sh b/ci.sh
index e32105b16..46354f096 100755
--- a/ci.sh
+++ b/ci.sh
@@ -169,6 +169,7 @@ cargo batch
--- build --release --manifest-path embassy-stm32/Cargo.toml --target thumbv6m-none-eabi --features stm32u031r8,defmt,exti,time-driver-any,time
--- build --release --manifest-path embassy-stm32/Cargo.toml --target thumbv6m-none-eabi --features stm32u073mb,defmt,exti,time-driver-any,time
--- build --release --manifest-path embassy-stm32/Cargo.toml --target thumbv6m-none-eabi --features stm32u083rc,defmt,exti,time-driver-any,time \

  • --- build --release --manifest-path embassy-stm32/Cargo.toml --target thumbv7em-none-eabihf --features stm32h7s3l8,defmt,time-driver-any,exti,unstable-pac
    --- build --release --manifest-path embassy-nxp/Cargo.toml --target thumbv8m.main-none-eabihf
    --- build --release --manifest-path cyw43/Cargo.toml --target thumbv6m-none-eabi --features ''
    --- build --release --manifest-path cyw43/Cargo.toml --target thumbv6m-none-eabi --features 'log' \
does not repro the bug when I execute `./ci.sh` on main.

@friedman-ionq friedman-ionq force-pushed the fix_stm32h7rs_building branch from 27dc230 to 37f995a Compare April 4, 2025 18:02
@Dirbaio
Copy link
Member

Dirbaio commented Apr 6, 2025

I cannot reproduce the build failure this is supposed to fix. Could you provide the exact build command to reproduce? What cargo build --features xxxxx do I have to run from the embassy-stm32 dir in a fresh repo clone?

Hertz values should always be printable with {}, if that's not working then we should fix that, not add .0 as a workaround.

@Dirbaio Dirbaio marked this pull request as draft April 6, 2025 21:27
@friedman-ionq
Copy link
Contributor Author

I cannot reproduce the build failure this is supposed to fix. Could you provide the exact build command to reproduce? What cargo build --features xxxxx do I have to run from the embassy-stm32 dir in a fresh repo clone?

Hertz values should always be printable with {}, if that's not working then we should fix that, not add .0 as a workaround.

I agree with that latter sentiment -- and then point out https://github.com/embassy-rs/embassy/blob/main/embassy-stm32/src/rcc/h.rs#L795 as an example where this workaround had previously been added. In any event, the command that I added to build.rs failed when I executed it in the embassy-stm32 directory.

@Dirbaio
Copy link
Member

Dirbaio commented Apr 7, 2025

can you try again to see if it fails now on latest main?

@friedman-ionq
Copy link
Contributor Author

can you try again to see if it fails now on latest main?

Now, it is not only passing on main, but also on 0.2. Perhaps it is breaking because of a dependency?

@Dirbaio
Copy link
Member

Dirbaio commented Apr 7, 2025

no idea! Since it works i'm going to close this, feel free to reopen if you do find the compile issue.

@Dirbaio Dirbaio closed this Apr 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants