Skip to content

SNS - Levitation Current Sensor#71

Open
Aux1r wants to merge 27 commits into
mainfrom
kacper/hype-92-implement-current-sensor-for-levitation
Open

SNS - Levitation Current Sensor#71
Aux1r wants to merge 27 commits into
mainfrom
kacper/hype-92-implement-current-sensor-for-levitation

Conversation

@Aux1r
Copy link
Copy Markdown
Contributor

@Aux1r Aux1r commented Feb 3, 2025

@Aux1r Aux1r requested a review from davidbeechey February 3, 2025 14:46
@Aux1r Aux1r self-assigned this Feb 3, 2025
@Aux1r Aux1r requested a review from H-Allen as a code owner February 3, 2025 14:46
@linear
Copy link
Copy Markdown

linear Bot commented Feb 3, 2025

@Aux1r Aux1r marked this pull request as draft February 3, 2025 14:46
@davidbeechey davidbeechey changed the title Create branch for levitation current sensor SNS - Levitation Current Sensor Feb 3, 2025
Copy link
Copy Markdown
Collaborator

@davidbeechey davidbeechey left a comment

Choose a reason for hiding this comment

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

Looks good, just a few things to tidy up

Comment thread lib/sensors/src/current_levitation.rs Outdated
Comment thread lib/sensors/src/current_levitation.rs Outdated
Comment thread lib/sensors/src/current_levitation.rs Outdated
Comment thread boards/stm32l432kc/src/tasks/current_levitation.rs Outdated
@davidbeechey davidbeechey added the needs testing Needs testing in the lab before merging label Feb 7, 2025
@Aux1r Aux1r marked this pull request as ready for review February 9, 2025 12:20
@Aux1r
Copy link
Copy Markdown
Contributor Author

Aux1r commented Feb 9, 2025

Rejigged the structure of the current_levitation.rs file and the current_levitation_test.rs files so that they're more akin to the temperature sensors' structure (do the matching for SensorValueRanges in the test instead of the task file)

Comment thread lib/sensors/src/current_levitation.rs Outdated
Comment thread lib/sensors/src/current_levitation.rs Outdated
@Aux1r Aux1r requested a review from jpfbastos as a code owner February 21, 2025 12:03
@davidbeechey davidbeechey added the needs reviews Needs reviews, otherwise finished label Feb 27, 2025
@davidbeechey davidbeechey added the blocked Something else is blocking progress label Mar 4, 2025
@Aux1r Aux1r requested a review from davidbeechey March 6, 2025 18:42
Copy link
Copy Markdown
Collaborator

@davidbeechey davidbeechey left a comment

Choose a reason for hiding this comment

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

Looks good, just waiting for testing now

Comment thread boards/stm32f767zi/src/bin/adc_test.rs Outdated
Comment thread boards/stm32l432kc/src/tasks/read_current_levitation.rs Outdated
@davidbeechey davidbeechey removed the needs reviews Needs reviews, otherwise finished label Mar 14, 2025
Copy link
Copy Markdown
Collaborator

@davidbeechey davidbeechey left a comment

Choose a reason for hiding this comment

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

I'm thinking we should rename this hall_effect rather than levitation_current? Makes it less application specific and more descriptive of the sensor itself

impl #impl_generics #name #ty_generics {
pub fn new(adc: Adc<'d, T>, channel: AnyAdcChannel<T>) -> Self {
Self { adc, channel }
/// Create a new instance of our ADC implementation for the STM32L476RG
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// Create a new instance of our ADC implementation for the STM32L476RG

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocked Something else is blocking progress needs testing Needs testing in the lab before merging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants