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

NonInvertingPin not implemented for OPAMP pins #3818

Open
teu5us opened this issue Jan 28, 2025 · 4 comments
Open

NonInvertingPin not implemented for OPAMP pins #3818

teu5us opened this issue Jan 28, 2025 · 4 comments

Comments

@teu5us
Copy link

teu5us commented Jan 28, 2025

The NonInvertingPin impls seem to be generated based on metapac here for pins with VP* signals, however, those seem to be VINM signals according to manual (stm32h743vi) and also according to the metapac. Is there anything I can do about it?

@Dirbaio
Copy link
Member

Dirbaio commented Jan 28, 2025

The issue comes from stm32-data.

See H7 has the pins with the VINP/VINM naming https://github.com/embassy-rs/stm32-data-generated/blob/main/data/chips/STM32H743VI.json#L4910

while G4 (where opamp pin impls do work) has VP0, VP1, .. https://github.com/embassy-rs/stm32-data-generated/blob/main/data/chips/STM32G491VE.json#L2611 . it also has the pins with the bad naming, and multiple times.

The fix should be:

  • Pick one naming
  • Make changes to stm32-data-gen so OPAMP pins are emitted with that naming (and only with that naming, not multiple. Each pin should only appear once)
  • Adjust embassy-stm32 build.rs to use the new naming (if it changed).

About which naming to pick, i'm not sure. I think the naming ST uses most commonly is VINPx/VINMx/VOUT ?

@teu5us
Copy link
Author

teu5us commented Jan 28, 2025

I'm not sure about the common naming as I'm new to this. VINP/VINM/VOUT feels right to me for some reason. InvertingPin is also not implemented, and I cannot find anything doing that in the sources as apposed to output pins and inverting pins. Besides, buffer_ext only takes NonInvertingPin as input where I can see inverting pin used in the C code that I have. So is there anything I can do right now to make this work?

@Dirbaio
Copy link
Member

Dirbaio commented Jan 28, 2025

for inverting pins you'd have to duplicate this to make the impls

embassy/embassy-stm32/build.rs

Lines 1231 to 1239 in f6532e8

if pin.signal.starts_with("VP") {
// Impl NonInvertingPin for the VP* signals (VP0, VP1, VP2, etc)
let peri = format_ident!("{}", p.name);
let pin_name = format_ident!("{}", pin.pin);
let ch: u8 = pin.signal.strip_prefix("VP").unwrap().parse().unwrap();
g.extend(quote! {
impl_opamp_vp_pin!( #peri, #pin_name, #ch);
})

and add new constructors to struct Opamp for use cases that use inverting pins.

@teu5us
Copy link
Author

teu5us commented Jan 28, 2025

G4 reference manual lists non-inverted inputs as VINP, I guess that it is the same for F3. I think that the naming should be kept as per the manual.

Correction: it is VM/VP for F3.

Image

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

No branches or pull requests

2 participants