pac extension traits and unmacro #3

burrbull · 2025-03-29T15:37:28Z

Only g4 for now.

usbalbin · 2025-04-13T22:18:25Z

This looks really, really nice from a quick glance! Let me know when you want me to take a closer look :)

burrbull · 2025-04-24T10:13:43Z

@usbalbin I don't think it is good idea to define pins here.
Here should be core part (structures and traits) which depends on PACs only.

In hals should be pin definitions and wrappers for initializers of structures defined here.
HAL depends on stm32-hrtim.
This repo should have HAL only in dev-dependencies.

usbalbin · 2025-04-24T10:38:39Z

Good idea! That makes sense

usbalbin · 2025-04-24T22:36:23Z

So something roughly like #6?

burrbull · 2025-04-25T03:28:40Z

Yes. It looks more reasonable for me.

burrbull · 2025-05-02T12:31:12Z

Rebased. But no guaranty there is nothing missed.

burrbull · 2025-06-14T06:38:04Z

rebased

usbalbin · 2025-06-15T20:45:55Z

Cargo.toml

+stm32f3 = { version = "0.16.0", optional = true }
+stm32h7 = { version = "0.16.0", optional = true }


usbalbin · 2025-06-15T21:05:53Z

src/ext.rs

+#[cfg(feature = "hrtim_v2")]
+use pac::hrtim_timf as timf;
+
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]


Suggested change

#[derive(Clone, Copy, Debug, PartialEq, Eq)]

#[derive(Clone, Copy, Debug, PartialEq, Eq)]

#[cfg_attr(feature = "defmt", derive(defmt::Format))]

Is it possible to derive defmt::Format for these?

usbalbin · 2025-06-15T21:31:13Z

src/timer.rs

+    F = 5,
+}
+
+pub trait Instance: Deref<Target = Self::RB> {


Would you mind writing some very brief doc comment? For example, what is the difference of Instance vs InstanceX? :)

The difference is that second one is not implemented for master timer. Although the names are not very good.

So something like

Suggested change

pub trait Instance: Deref<Target = Self::RB> {

/// Any HR Timer A-F or Master

pub trait Instance: Deref<Target = Self::RB> {

usbalbin · 2025-06-15T21:34:08Z

src/timer.rs

+        let common = unsafe { &*HRTIM_COMMON::ptr() };
+        common.cr1().modify(|_, w| match TIM::TIMX {
+            Timer::Master => w.mudis().clear_bit(),
+            Timer::Tim(v) => w.tudis(v as _).clear_bit(),


This really is very nice!

usbalbin · 2025-06-15T21:36:29Z

src/ext.rs

+        pub trait $TrR {
+            $(fn $f(&self $(, $n: $e)?) -> $fr;)*
+        }
+        impl<REG: reg::$TrR> $TrR for R<REG> {


Just curious, what does these do?

Is this somehow to abstract away the calls to the different functions of all timers?

Just curious, what does these do?

What is the question about? The goal of these wrapper traits?

Maybe it is not the best solution but the best that I've found to abstract over similar but not identical registers across different peripherals. It requires fields to be reexported (derived in SVD) to work.

The idea is to use unique REGrs register specification as associated type.
In current implementation you need 4 traits for each RW register: 2 traits for read and write implemented on REGrs + 1 wrapper for R<REGrs> and 1 wrapper for W<REGrs.

Alternative options:

It can work without wrapper traits but you need to call it as path_to::REGrs::field(&r) or something like this. Which is annoying and different syntax. Works but not good.

You could make implementations directly on R<REGrs> and W<REGrs. But in this case rust can't infer types and require where everywhere. Very bad.

But option 3=1+2 works.
The magic is in impl<REG: reg::RegRead> RegRead for R<REG> line.

Something like

trait Instance { type REG: RegRead + RegWrite; // it can be single `Reg` trait }

allows to use canonical svd2rust API (reg.write(|w| w.field().variant()), reg.read().field().variant()) for any generic PER: Instance peripheral.

I can not quite say I understand all implications of the above, but does seem to make a lot of sense :)

Thank you for the explanation

usbalbin

This is very nice! Thanks a lot for this! :)

Since this changes quite a bit of(most) things, I would like to run some tests on some hardware just to confirm that at least some of the most basic things still work. I will try to do this in the coming days, hopefully.

In the meantime, CI seems to be a bit angry about capture.rs.

burrbull · 2025-06-16T02:12:16Z

In the meantime, CI seems to be a bit angry about capture.rs.

stm32g4xx-hal hrtim branch depends on wrong stm32-hrtim.

Not sure how to solve this. Maybe use some script to add patch section in Cargo.toml in CI?

burrbull · 2025-06-16T03:39:32Z

Why CI does not run automatically?

usbalbin · 2025-06-16T05:41:10Z

Why CI does not run automatically?

Correct me if I am wrong but I think a user gets "approved" for CI only after having their first PR merged

stm32g4xx-hal hrtim branch depends on wrong stm32-hrtim.

Oh sorry did not realise that

usbalbin · 2025-06-16T06:20:46Z

If you want to not count as a first time contributor, you could probably open a PR adding a dot or something to the README

usbalbin · 2025-06-20T22:46:45Z

I ran the tests added in stm32-rs/stm32g4xx-hal#192 for this branch and they pass. They dont test much but at least it is something :)

I am happy to merge this now unless you have something more to add

burrbull · 2025-06-21T02:32:15Z

I am happy to merge this now unless you have something more to add

Nothing for now. At least until new stm32-rs release.

usbalbin · 2025-06-21T08:52:04Z

Thank you!

This was referenced Apr 26, 2025

HRTIM stm32-rs/stm32g4xx-hal#192

Merged

Dont depend on hal #6

Merged

usbalbin changed the base branch from the_code to main May 2, 2025 11:21

burrbull force-pushed the ext branch from 8166c6c to 8e8d232 Compare May 2, 2025 12:25

burrbull force-pushed the ext branch 3 times, most recently from 17d4dcc to 3697fff Compare June 14, 2025 06:37

burrbull marked this pull request as ready for review June 14, 2025 06:37

usbalbin reviewed Jun 15, 2025

View reviewed changes

burrbull force-pushed the ext branch from b473daf to e33dc62 Compare June 16, 2025 14:59

ext

14384fc

burrbull force-pushed the ext branch from e33dc62 to 14384fc Compare June 17, 2025 01:45

usbalbin merged commit 558357b into usbalbin:main Jun 21, 2025
10 checks passed

		stm32f3 = { version = "0.16.0", optional = true }
		stm32h7 = { version = "0.16.0", optional = true }

	#[derive(Clone, Copy, Debug, PartialEq, Eq)]
	#[derive(Clone, Copy, Debug, PartialEq, Eq)]
	#[cfg_attr(feature = "defmt", derive(defmt::Format))]

	pub trait Instance: Deref<Target = Self::RB> {
	/// Any HR Timer A-F or Master
	pub trait Instance: Deref<Target = Self::RB> {

pac extension traits and unmacro #3

pac extension traits and unmacro #3

Uh oh!

Conversation

burrbull commented Mar 29, 2025

Uh oh!

usbalbin commented Apr 13, 2025

Uh oh!

burrbull commented Apr 24, 2025

Uh oh!

usbalbin commented Apr 24, 2025

Uh oh!

usbalbin commented Apr 24, 2025

Uh oh!

burrbull commented Apr 25, 2025

Uh oh!

burrbull commented May 2, 2025

Uh oh!

burrbull commented Jun 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

usbalbin Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

usbalbin left a comment

Choose a reason for hiding this comment

Uh oh!

burrbull commented Jun 16, 2025

Uh oh!

burrbull commented Jun 16, 2025

Uh oh!

usbalbin commented Jun 16, 2025

Uh oh!

usbalbin commented Jun 16, 2025

Uh oh!

usbalbin commented Jun 20, 2025

Uh oh!

burrbull commented Jun 21, 2025

Uh oh!

Uh oh!

usbalbin commented Jun 21, 2025

Uh oh!

Uh oh!

usbalbin Jun 20, 2025 •

edited

Loading