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

CI #92

Merged
merged 8 commits into from
Dec 16, 2023
Merged

CI #92

merged 8 commits into from
Dec 16, 2023

Conversation

usbalbin
Copy link
Contributor

  • Add g491 and 4a1 to ci, check examples, check with different feature sets

  • Remove unused feature embedded-can-03

  • Remove feature unstable-defmt from ci

  • Add clippy to ci

  • Fix warning

@usbalbin usbalbin changed the title Ci2 (#6) CI Nov 29, 2023
no111u3
no111u3 previously approved these changes Nov 29, 2023
@no111u3 no111u3 dismissed their stale review November 29, 2023 20:23

Fail on CI

@usbalbin
Copy link
Contributor Author

I might have resolved those remaining warnings somewhere in my messy branch with too many changes. Will try to take a look later..

* Add g491 and 4a1 to ci, check examples, check with different feature sets

* Add todo

* Remove unused feature embedded-can-03

* Remove feature unstable-defmt from ci

* Add clippy to ci

* Fix warning

* g91 and 4a1 support does not seem quite ready yet
@usbalbin
Copy link
Contributor Author

Seems like log-itm's println requires a logger as parameter.

error: unexpected end of macro invocation
  --> examples/button.rs:80:26
   |
80 |     println!("Start Loop");
   |                          ^ missing tokens in macro arguments
   |
note: while trying to match `,`
  --> /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cortex-m-log-0.7.0/src/lib.rs:59:18
   |
59 |     ($logger:expr, $($arg:tt)+) => ({
   |                  ^

None of the examples are made with that in mind.

Any advice on how to proceed?

@usbalbin
Copy link
Contributor Author

Btw, https://github.com/DoumanAsh/cortex-m-log seems to be not so active. Not sure how many users use that vs rtt or semihosting

@usbalbin
Copy link
Contributor Author

usbalbin commented Dec 3, 2023

As I see it, we have atleast two options:

  1. Change utils::logger::init() to return the logger instance (if or () where not applicable) and add $logger as the first argument to all println impl's to make the syntax identical
  2. Remove cortex-m-log as an option for our examples

@no111u3 do you have any suggestions? :)

@usbalbin
Copy link
Contributor Author

usbalbin commented Dec 16, 2023

As I see it, we have atleast two options:

  1. Change utils::logger::init() to return the logger instance (if or () where not applicable) and add $logger as the first argument to all println impl's to make the syntax identical
  2. Remove cortex-m-log as an option for our examples

@no111u3 do you have any suggestions? :)

It was possible to work around the issue by making println use log::info which does not need a logger instance (assuming logger::init has been called. So now CI passes!

This PR should now be ready for review :)

@no111u3 no111u3 merged commit a33b9fc into stm32-rs:main Dec 16, 2023
23 checks passed
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