Skip to content

Conversation

@bugadani
Copy link
Contributor

@bugadani bugadani commented Sep 21, 2023

This PR:

  • removes duplicate example checks
  • adds stuff to make sure the defmt feature not only passes cargo check but actually builds
  • adds +nightly toolchain spec for consistency
  • don't enable esp-println/log by default
  • fixes examples to work with log disabled

@bugadani
Copy link
Contributor Author

Turns out, many of the current examples (that are checked by CI) don't actually link. Part of the problem is defmt, which is fixed by this PR (though I'm still very much confused why e-h-common can build at all, even though Interrupts doesn't implement Format yet...?), but other than this, the timer and sw interrupt examples aren't linking if embassy support is enabled, due to multiple definitions of certain interrupt handlers. How should we handle these examples?

@bugadani bugadani marked this pull request as draft September 21, 2023 16:35
@bugadani bugadani marked this pull request as ready for review September 21, 2023 16:40
@bugadani bugadani force-pushed the defmt branch 2 times, most recently from c8e8199 to 7ed4e34 Compare September 28, 2023 08:11
@bugadani bugadani mentioned this pull request Sep 28, 2023
@bjoernQ
Copy link
Contributor

bjoernQ commented Sep 29, 2023

looks good to me but I'd like to leave the final word on this to @jessebraham since he did most of the work on CI

Copy link
Member

@jessebraham jessebraham left a comment

Choose a reason for hiding this comment

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

Sorry for the delay in getting to this, thanks for making that change. LGTM!

@bugadani
Copy link
Contributor Author

bugadani commented Oct 5, 2023

Hold on please, things don't build locally.

@jessebraham
Copy link
Member

At this point I'm getting pretty concerned that we've added this defmt feature and nobody can seem to get CI to verify it.

@bugadani
Copy link
Contributor Author

bugadani commented Oct 5, 2023

This PR wasn't trying to get CI to verify the feature, but I can add a cargo build for each build target if you want me to.

@jessebraham
Copy link
Member

jessebraham commented Oct 5, 2023

The title of the PR is Make sure examples can actually build with defmt enabled, CI is green, and you're saying it's not building. Maybe I'm misunderstanding but this doesn't seem right.

@bugadani
Copy link
Contributor Author

bugadani commented Oct 5, 2023

Yeah, this is a fix to build the examples locally. I didn't want to increase CI times by turning some cargo checks into cargo builds.

@bugadani bugadani marked this pull request as draft October 5, 2023 16:51
@bugadani bugadani marked this pull request as ready for review October 5, 2023 17:08
@bugadani
Copy link
Contributor Author

bugadani commented Oct 5, 2023

@jessebraham if you're fine with cargo build this is fine now. Looks like the linker script needs to be added to each package. I'm suspecting something's up with conditional compilation and build scripts.

@bugadani bugadani changed the title Make sure examples can actually build with defmt enabled Make sure examples can actually build with defmt enabled, build some examples in CI Oct 5, 2023
@jessebraham jessebraham merged commit 44e968f into esp-rs:main Oct 5, 2023
@bugadani bugadani deleted the defmt branch October 5, 2023 19:23
MabezDev pushed a commit to MabezDev/esp-hal that referenced this pull request Oct 20, 2023
…examples in CI (esp-rs#810)

* Make sure examples can build with defmt

* Remove duplicate example checks

* Fix examples

* Add changelog entry

* Actually build some examples with defmt feature enabled

* Add the defmt linker script in each package's build script
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.

3 participants