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

IMU refactor #51

Closed
wants to merge 18 commits into from
Closed

IMU refactor #51

wants to merge 18 commits into from

Conversation

guyfleeman
Copy link
Contributor

Fixes #46

raw_sample as f32 * (conversion_num / i16::MAX as f32)
}

pub async fn gyro_get_data(&mut self) -> [f32; 3] {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would we want this be rad/s? Or maybe name the function so that it's clear it returns deg/s?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added accessor functions for both units and added units as postfix to each function name

let gyro_pub = GYRO_CHANNEL.publisher().unwrap();
let gyro_sub = GYRO_CHANNEL.subscriber().unwrap();

_spawner.spawn(imu_task(gyro_pub, accel_pub, p.SPI6, p.PA5, p.PA7, p.PA6, p.BDMA_CH0, p.BDMA_CH1, p.PC4, p.PC5, p.PB1, p.PB2, p.EXTI1, p.EXTI2)).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

The task seems to default using from_pins constructor for the IMU. So I think imu_spi in this file will have issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in the latest commit


let imu_buf = IMU_BUFFER_CELL.init([0; 8]);
let mut imu = Bmi085::new_from_pins(imu_spi, sck, mosi, miso, txdma, rxdma, accel_cs, gyro_cs, imu_buf);

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add the self_test here? Related but accel needs to have a dummy read to initialize to SPI mode and self_test does that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

created an init function to be called. I don't like having new() be async because that breaks some meta patterns in Rust that are seemingly unofficial, but still.

@guyfleeman
Copy link
Contributor Author

keep having a bad habit of doing this but it's def a Rust anti pattern. Do not use bool for function return status. Using Result<(), ()> or specify result sub types. Fix before merge.

@guyfleeman
Copy link
Contributor Author

keep having a bad habit of doing this but it's def a Rust anti pattern. Do not use bool for function return status. Using Result<(), ()> or specify result sub types. Fix before merge.

addressed this as well

@guyfleeman guyfleeman closed this Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IMU Updates
3 participants