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

Add Comparator and DAC #74

Merged
merged 17 commits into from
Nov 29, 2023
Merged

Add Comparator and DAC #74

merged 17 commits into from
Nov 29, 2023

Conversation

usbalbin
Copy link
Contributor

@usbalbin usbalbin commented May 31, 2023

Add modified src/dac.rs and src/comparator.rs from g0xx-hal. Also added support for internal connection between DAC and comparator.

Tested on nucleo-g474re (stm32g474re)

let mut delay = cp.SYST.delay(&rcc.clocks);

let gpioa = dp.GPIOA.split(&mut rcc);
let dac1ch1 = dp.DAC1.constrain((gpioa.pa4, Dac1IntSig1), &mut rcc);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure how this should best be done.. There is no way for the user to know whether to specify Dac1IntSig1 or Dac1IntSig2(besides a not too nice compile error) in order to create a dac in with a mode compatible with the comparator.

Suggestions welcome :)

Comment on lines 143 to 155
/*
pub fn power_mode(mut self, power_mode: PowerMode) -> Self {
self.power_mode = power_mode;
self
}*/

/*
/// Sets the output to be Comparator 1 XOR Comparator 2.
/// Used to implement window comparator mode.
pub fn output_xor(mut self) -> Self {
self.output_xor = true;
self
}*/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAIK this is not supported by stm32g4 processors. Should this be removed?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes we should. We don't need to store code in comments.

/*
/// Uses two comparators to implement a window comparator.
/// See Figure 69 in RM0444 Rev 5.
pub struct WindowComparator<U, L, ED> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same thing with window mode, not supported

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also remove every inapplicable code.

src/dac.rs Outdated
Comment on lines 194 to 203
/*pub fn enable_unbuffered(self) -> $CX<MODE_BITS, EnabledUnbuffered> {
let dac = unsafe { &(*<$DAC>::ptr()) };

dac.dac_mcr.modify(|_, w| unsafe { w.$mode().bits(2) });
dac.dac_cr.modify(|_, w| w.$en().set_bit());

$CX {
_enabled: PhantomData,
}
}*/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Buffered vs unbuffered is pretty much chosen by MODE_BITS. Not sure how we want to handle that

Copy link
Collaborator

Choose a reason for hiding this comment

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

Use input argument as selector between modes

@usbalbin usbalbin marked this pull request as ready for review June 2, 2023 18:42
@no111u3
Copy link
Collaborator

no111u3 commented Nov 28, 2023

@usbalbin please update the PR if it still needed.

src/opamp.rs Outdated
@@ -45,6 +45,8 @@
//! }
//! ```

// TODO: Add support for locking using the `LOCK` bit in `OPAMPx_CSR`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Needs to update to current main branch

src/dac.rs Outdated
Comment on lines 194 to 203
/*pub fn enable_unbuffered(self) -> $CX<MODE_BITS, EnabledUnbuffered> {
let dac = unsafe { &(*<$DAC>::ptr()) };

dac.dac_mcr.modify(|_, w| unsafe { w.$mode().bits(2) });
dac.dac_cr.modify(|_, w| w.$en().set_bit());

$CX {
_enabled: PhantomData,
}
}*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use input argument as selector between modes

Comment on lines 143 to 155
/*
pub fn power_mode(mut self, power_mode: PowerMode) -> Self {
self.power_mode = power_mode;
self
}*/

/*
/// Sets the output to be Comparator 1 XOR Comparator 2.
/// Used to implement window comparator mode.
pub fn output_xor(mut self) -> Self {
self.output_xor = true;
self
}*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes we should. We don't need to store code in comments.

/*
/// Uses two comparators to implement a window comparator.
/// See Figure 69 in RM0444 Rev 5.
pub struct WindowComparator<U, L, ED> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also remove every inapplicable code.

}

// TODO: look up alternate functions for more devices than g474
// https://www.mouser.se/datasheet/2/389/stm32g474cb-1600828.pdf#page=73
Copy link
Collaborator

Choose a reason for hiding this comment

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

please use the documents from ST site only. also use their public id like as ANxxx, RMxxx, etc.

fn setup(&self, comp: &C);
}

/*
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove the unused or inapplicable code.

@usbalbin
Copy link
Contributor Author

I believe I have resolved your comments. Btw thanks a lot :)

@no111u3
Copy link
Collaborator

no111u3 commented Nov 29, 2023

@usbalbin thank you so much. I've to merge it.

@no111u3 no111u3 merged commit a92f938 into stm32-rs:main Nov 29, 2023
9 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