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

SNS - I2C ToF #26

Merged
merged 60 commits into from
Jan 20, 2025
Merged

SNS - I2C ToF #26

merged 60 commits into from
Jan 20, 2025

Conversation

Aux1r
Copy link
Contributor

@Aux1r Aux1r commented Oct 14, 2024

initial commit for time of flight sensor code, hope the folder structure is okay

Copy link

linear bot commented Oct 14, 2024

@Aux1r Aux1r changed the title initial commit SNS - I2C ToF Oct 14, 2024
@davidbeechey davidbeechey marked this pull request as draft October 15, 2024 10:17
@Aux1r
Copy link
Contributor Author

Aux1r commented Oct 21, 2024

Datasheet available here for the VL6180V1 ToF Sensor: https://www.st.com/en/imaging-and-photonics-solutions/vl6180.html#overview

@Aux1r Aux1r changed the title SNS - I2C ToF SNS - I2C ToF (HYPE-27) Oct 21, 2024
@davidbeechey davidbeechey changed the title SNS - I2C ToF (HYPE-27) SNS - I2C ToF Oct 31, 2024
@Aux1r
Copy link
Contributor Author

Aux1r commented Oct 31, 2024

  • Added functions for i2c trait to allow reading and writing to registers with 16 bit addresses.
  • Created new() function, start_ss_measure() function which starts a singleshot measurement, poll_range() function that polls to see if a new range reslt is available, read_range() that reads this value.

A lot of these functions were implemented using the 'example code' provided in the VL6180 application sheet: https://www.st.com/resource/en/application_note/an4545-vl6180x-basic-ranging-application-note-stmicroelectronics.pdf

TODO:

  • Implement continuous mode measurements
  • Verify decoding of RESULT_RANGE_VAL is appropriate (and actually returns us the value in mm)
  • Verify poll_range() function works (not sure how Rust likes the while loop)
  • Add clear_interrupts() function and checks for if the system is 'fresh out of reset' (refer to Application Sheet)

lib/io/src/i2c.rs Outdated Show resolved Hide resolved
lib/sensors/src/tof.rs Outdated Show resolved Hide resolved
@Aux1r Aux1r marked this pull request as ready for review November 14, 2024 18:35
@davidbeechey davidbeechey added needs-testing Needs testing in the lab before merging needs-reviews Needs reviews, otherwise finished labels Nov 15, 2024
@Aux1r Aux1r requested a review from davidbeechey November 16, 2024 15:43
Tests are failing though, not sure why
Copy link
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.

This is very nearly done now! The tests aren't working properly (not sure why). A few comments here to fix and think about the API. Apart from those functionality points, would be good to add Rust doc comments to all of your functions

boards/stm32l476rg/src/tasks/tof.rs Outdated Show resolved Hide resolved
lib/sensors/src/tof.rs Outdated Show resolved Hide resolved
lib/sensors/src/tof.rs Outdated Show resolved Hide resolved
lib/sensors/src/tof.rs Outdated Show resolved Hide resolved
lib/sensors/src/tof.rs Outdated Show resolved Hide resolved
lib/sensors/src/tof.rs Outdated Show resolved Hide resolved
lib/sensors/src/tof.rs Outdated Show resolved Hide resolved
lib/sensors/src/tof.rs Outdated Show resolved Hide resolved
@davidbeechey davidbeechey removed the needs-testing Needs testing in the lab before merging label Nov 20, 2024
Copy link

@TomLonergan03 TomLonergan03 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. i didn't read the datasheet so haven't commented on correctness stuff there

2 things:

  • in general i think you should rename the "tof"s in file names and code to be "time_of_flight", just for sake of clarity.
  • a lot of the TimeOfFlight impl methods are kinda unclear on what they do, so there's probably better names for them or if not add doc comments

lib/io/src/i2c.rs Outdated Show resolved Hide resolved
lib/sensors/src/tof.rs Outdated Show resolved Hide resolved
lib/sensors/src/tof.rs Outdated Show resolved Hide resolved
lib/sensors/src/tof.rs Outdated Show resolved Hide resolved
lib/sensors/src/tof.rs Outdated Show resolved Hide resolved
lib/sensors/src/tof.rs Outdated Show resolved Hide resolved
lib/sensors/src/tof.rs Outdated Show resolved Hide resolved
lib/sensors/src/tof.rs Outdated Show resolved Hide resolved
lib/sensors/src/tof.rs Outdated Show resolved Hide resolved
Copy link

@TomLonergan03 TomLonergan03 left a comment

Choose a reason for hiding this comment

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

looks very good, couple of notes

boards/stm32l476rg/src/tasks/time_of_flight.rs Outdated Show resolved Hide resolved
lib/sensors/src/time_of_flight.rs Outdated Show resolved Hide resolved
lib/sensors/src/time_of_flight.rs Outdated Show resolved Hide resolved
lib/sensors/src/time_of_flight.rs Outdated Show resolved Hide resolved
@davidbeechey davidbeechey removed the needs-reviews Needs reviews, otherwise finished label Jan 16, 2025
@davidbeechey davidbeechey merged commit c403a26 into main Jan 20, 2025
15 checks passed
@davidbeechey davidbeechey deleted the kacper/hype-27-implement-time-of-flight branch January 20, 2025 19:39
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.

4 participants