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.

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

Draft: Adding defmt-1.0 #909

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Draft: Adding defmt-1.0 #909

wants to merge 5 commits into from

Conversation

jonathanpallant
Copy link
Contributor

@jonathanpallant jonathanpallant commented Dec 3, 2024

Adds a defmt 0.3 proxy, and bumps most crates to 1.0.0-alpha.

Tested by patching the radio-app from the exercises to use the defmt-0.3 folder. That seems to work.

diff --git a/nrf52-code/radio-app/Cargo.toml b/nrf52-code/radio-app/Cargo.toml
index 78649ece..e959cfdd 100644
--- a/nrf52-code/radio-app/Cargo.toml
+++ b/nrf52-code/radio-app/Cargo.toml
@@ -33,3 +33,6 @@ incremental = false
 lto = "fat"
 opt-level = 3
 overflow-checks = false
+
+[patch.crates-io]
+defmt = { path = "/Users/jonathan/Documents/knurling-rs/defmt/defmt-03" }
diff --git a/nrf52-code/radio-app/src/bin/hello.rs b/nrf52-code/radio-app/src/bin/hello.rs
index 104bd832..60e6c3bd 100644
--- a/nrf52-code/radio-app/src/bin/hello.rs
+++ b/nrf52-code/radio-app/src/bin/hello.rs
@@ -8,6 +8,12 @@ use cortex_m_rt::entry;
 // this imports `src/lib.rs`to retrieve our global logger + panicking-behavior
 use radio_app as _;

+#[derive(defmt::Format)]
+struct Oblong {
+    width: u32,
+    height: u32
+}
+
 // the custom entry point
 // 👇🏾
 #[entry]
@@ -18,7 +24,8 @@ fn main() -> ! {
     // initializes the peripherals
     dk::init().unwrap();

-    defmt::println!("Hello, world!"); // 👋🏾
+    let ob = Oblong { width: 10, height: 20 };
+    defmt::println!("Hello, world! sq = {:?}", ob); // 👋🏾

     dk::exit();
 }
$ cargo run --bin hello -- --probe 1366:1051:001050288864
    Finished `dev` profile [optimized + debuginfo] target(s) in 0.14s
     Running `probe-rs run --chip nRF52840_xxAA --allow-erase-all target/thumbv7em-none-eabihf/debug/hello --probe '1366:1051:001050288864'`
      Erasing ✔ [00:00:00] [####################################################################################################################################################################################################] 8.00 KiB/8.00 KiB @ 29.09 KiB/s (eta 0s )
  Programming ✔ [00:00:00] [####################################################################################################################################################################################################] 8.00 KiB/8.00 KiB @ 24.74 KiB/s (eta 0s )    Finished in 0.628s
<lvl> Hello, world! sq = Oblong { width: 10, height: 20 }
└─ hello::__cortex_m_rt_main @ src/bin/hello.rs:28
<lvl> `dk::exit()` called; exiting ...
└─ dk::exit @ /Users/jonathan/Documents/ferrous-systems/rust-exercises/nrf52-code/boards/dk/src/lib.rs:415

@jonathanpallant
Copy link
Contributor Author

ahh, changelogs.

@jonathanpallant
Copy link
Contributor Author

error: package defmt is ambiguous: it is defined by in multiple manifests within the root path /home/runner/work/defmt/defmt

Not sure how to make cargo-semver-checks happy here.

@Urhengulas
Copy link
Member

error: package defmt is ambiguous: it is defined by in multiple manifests within the root path /home/runner/work/defmt/defmt

Not sure how to make cargo-semver-checks happy here.

Maybe you can exclude it?

@Urhengulas
Copy link
Member

Executing cargo semver-checks in the respective directory works.

$ cargo semver-checks --exclude defmt
<...>
$ cd defmt/
$ cargo semver-checks --default-features
     Parsing defmt v1.0.0-alpha (current)
      Parsed [   5.198s] (current)
     Parsing defmt v0.3.10 (baseline)
      Parsed [   5.530s] (baseline)
    Checking defmt v0.3.10 -> v1.0.0-alpha (major change)
     Checked [   0.000s] 0 checks: 0 pass, 94 skip
     Summary no semver update required
    Finished [  10.738s] defmt
$ cd ../defmt-03
$ cargo semver-checks --default-features
     Parsing defmt v0.3.100 (current)
      Parsed [   5.936s] (current)
     Parsing defmt v0.3.10 (baseline)
      Parsed [   5.592s] (baseline)
    Checking defmt v0.3.10 -> v0.3.100 (minor change)
     Checked [   0.006s] 87 checks: 84 pass, 3 fail, 0 warn, 7 skip
<checks all crates except the two defmt ones>

This should work (not tested):

diff --git a/.github/workflows/cargo-semver-check.yml b/.github/workflows/cargo-semver-check.yml
index b86b92c..0cbbf44 100644
--- a/.github/workflows/cargo-semver-check.yml
+++ b/.github/workflows/cargo-semver-check.yml
@@ -14,10 +14,20 @@ jobs:
 
     steps:
       - uses: actions/checkout@v4
-      - name: Semver check host crates
+      - name: Semver check host crates, except defmt
+        uses: obi1kenobi/cargo-semver-checks-action@v2
+        with:
+          exclude: defmt
+      - name: Semver check defmt v1
+        uses: obi1kenobi/cargo-semver-checks-action@v2
+        with:
+          feature-group: default-features
+          manifest-path: defmt/
+      - name: Semver check defmt v0.3
         uses: obi1kenobi/cargo-semver-checks-action@v2
         with:
           feature-group: default-features
+          manifest-path: defmt-03/
       - name: Semver check firmware crates
         uses: obi1kenobi/cargo-semver-checks-action@v2
         with:

Copy link

cloudflare-workers-and-pages bot commented Dec 9, 2024

Deploying knurling-defmt-book with  Cloudflare Pages  Cloudflare Pages

Latest commit: 03de6a3
Status: ✅  Deploy successful!
Preview URL: https://c6029ee6.knurling-defmt-book.pages.dev
Branch Preview URL: https://defmt-1-0.knurling-defmt-book.pages.dev

View logs

@jonathanpallant jonathanpallant force-pushed the defmt-1.0 branch 3 times, most recently from f43e326 to 3ea91f5 Compare December 12, 2024 11:31
It doesn't like having two versions of defmt, and it ignores the 'exclude' field in the workspace.
…and pub use re-exports.

It's something Predrag is working on, but for now, we have to do this by hand.
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