SNS - Time-of-flight Sensor by RheaBose · Pull Request #25 · Hyp-ed/hyped-2024

RheaBose · 2023-11-06T18:35:22Z

Implemented with reference to STMicroelectronics application note AN4545

Rebase with master after SNS - 16-Bit I2C #58 has been merged
Confirm requirements for what this sensor should do (Is ambient light reading required? Continuous or single-shot?)
Implement initialisation of ToF, including registers without names
Implement reading continuous range
Implement reading single-shot range
Implement II2cMuxSensor
Test on hardware

DylanCavers

A few changes, mostly to do with formatting and code style, make sure to change the tof file to tof.cpp

TomLonergan03

also, run clang-format over this

TomLonergan03

Rename this PR also

sa-fx · 2024-01-28T00:11:47Z

Implemented with reference to STMicroelectronics application note AN4545

Task list moved to top of PR

sa-fx · 2024-01-28T01:00:17Z

+
+ private:
+  // TODOLater - Confirm these addresses are correct
+  // Register addresses/values taken from the VL6180X datasheet


Not sure where exactly they've been taken from, or what they all mean - Are these needed?

sa-fx · 2024-01-28T01:03:18Z

+  static constexpr std::uint16_t kReadoutSamplingPeriod = 0x010a;
+  static constexpr std::uint8_t kSysAlsAnalogueGain     = 0x03f;
+  static constexpr std::uint8_t kSysRangeVhvRepeatRate  = 0x031;
+  // TODOLater - Confirm SYSALS__INTEGRATION_PERIOD (application note vs datasheet)


The datasheet gives this register an address of 0x040, but the application note AN4545 gives this register an address of 0x0041 on page 24

lmfao of course it does

engineers write a useful datasheet challenge (impossible)

TomLonergan03

ik it's a wip, but here's critique anyway lol

TomLonergan03 · 2024-01-28T01:01:33Z

+std::optional<std::uint8_t> TimeOfFlight::getRangeSingleShot()
+{
+  if (checkSensorMode(kModeSingleShot) == core::Result::kFailure) { return std::nullopt; }
+  return getRange();
+};
+
+std::optional<std::uint8_t> TimeOfFlight::getRangeContinuous()
+{
+  if (checkSensorMode(kModeContinuous) == core::Result::kFailure) { return std::nullopt; }
+  return getRange();
+};


what are these for?

The sensor has two modes of operation: Single shot, and continuous. Single shot is a single measurement, while continuous will repeatedly measure the range with a time interval based on the value of kSysRangeIntermeasurementPeriod. Before reading in a given mode, register kSysRangeStart has to be set to either 1 or 3
Because the code to check that kSysRangeStart has the correct value is the same whether its checking for continuous or single-shot, and because the register to read the range from is the same in both cases if kSysRangeStart has the correct value (kResultRangeVal), getRangeSingleShot() and getRangeContinuous() call these separate functions to retrieve the range

As for which value will be wanted, single shot or continous, that's yet to be decided

TomLonergan03 · 2024-01-28T01:03:56Z

+  auto range_status = *status & 0b111;
+
+  // Wait for new measurement ready status
+  while (range_status != 0x04) {


this is scary, we need to be able to guarantee that it can't get stuck in this loop

sa-fx · 2024-05-02T08:23:43Z

-    i2c_->writeByteToRegister(device_address_, array_return_addresses[15], 0x3c);
-    i2c_->writeByteToRegister(device_address_, array_return_addresses[16], 0x09);
-    i2c_->writeByteToRegister(device_address_, array_return_addresses[17], 0x09);
-    // i2c_->writeByteToRegister(device_address_, 0x0198, 0x01);


These registers were not commented out because they should be removed. They were commented out because at the time this code was written, #58 hadn't been merged yet. As such, the 16-bit registers being written to would cause build failure.
These registers should be added back in. Refer to the application note in the PR's header

sa-fx · 2024-05-02T08:25:42Z

  while (range_status != 0x04) {
-    status = i2c_->readByte(device_address_, kResultInterruptStatusGpio);
-    if (!status) {
+    const auto statuss = i2c_->readByte(device_address_, kResultInterruptStatusGpio);


statuss is quite the variable name.

sa-fx · 2024-05-02T08:26:17Z

  auto range_status = *status & 0b111;

  // Wait for new measurement ready status
+  uint8_t timeout = 0;


std::uint8_t

sa-fx · 2024-05-02T08:27:08Z

-    range_status = *status & 0b111;
+    range_status = *statuss & 0b111;
+    timeout++;
+    if (timeout > 1000000) {


Comparing a uint8_t variable to 1,000,000 will always return false

sa-fx · 2024-05-02T08:28:18Z

-    range_status = *status & 0b111;
+    range_status = *statuss & 0b111;
+    timeout++;
+    if (timeout > 1000000) {


Is 1,000,000 sensible? It seems quite large.

sa-fx · 2024-05-02T08:36:54Z


    // Recommended : Public registers
    i2c_->writeByteToRegister(device_address_, kSystemModeGpioOne, 0x10);
-    // i2c_->writeByteToRegister(device_address_, kReadoutSamplingPeriod, 0x30);


Again, this was a 16-bit register which would cause build failure, not a line which needed to be removed. It should be added back in

sa-fx · 2024-05-02T08:43:53Z

-    // i2c_->writeByteToRegister(device_address_, 0x01a7, 0x1f);
-    i2c_->writeByteToRegister(device_address_, array_return_addresses[19], 0x00);
-
+    constexpr std::uint8_t data[] = {0xfd,


Add entries to this for the values to be written to the 16-bit registers

sa-fx · 2024-05-02T08:45:44Z

@@ -1,4 +1,4 @@
-#include "time_of_flight.hpp"
+#include "tof.hpp"


Why was this changed back?

sa-fx · 2024-05-02T08:48:54Z

Make array_return_addresses a std::uint16_t array, and add the 16-bit registers. Then you can itterate through all the registers
(AN4545, p20)

sa-fx · 2024-05-14T13:31:59Z

You should delete these tof.cpp/hpp files.

sa-fx · 2024-05-14T13:38:44Z

+
+  // Wait for new measurement ready status
+  std::uint8_t timeout = 0;
+  std::uint8_t threshold = 10000;


std::unit8_t has a max value of 255. If you want to use 10,000 here, you'll need std::uint16_t, and you'll also need timeout to be std::uint16_t for the comparison to work

sa-fx · 2024-05-14T13:41:42Z

+
+  // Wait for new measurement ready status
+  std::uint8_t timeout = 0;
+  std::uint8_t threshold = 10000;


Where has the value of 10,000 come from? What's the rationale for choosing that value?

Add files via upload

5532a5c

RheaBose requested a review from licornes-fluos as a code owner November 6, 2023 18:35

RheaBose self-assigned this Nov 6, 2023

RheaBose requested review from DylanCavers, TomLonergan03 and davidbeechey November 6, 2023 18:47

DylanCavers reviewed Nov 6, 2023

View reviewed changes

Comment thread lib/sensors/tof Outdated

Comment thread lib/sensors/tof Outdated

Comment thread lib/sensors/tof Outdated

Comment thread lib/sensors/tof Outdated

Comment thread lib/sensors/tof.hpp Outdated

Comment thread lib/sensors/tof.hpp Outdated

Comment thread lib/sensors/tof Outdated

TomLonergan03 requested changes Nov 6, 2023

View reviewed changes

Comment thread lib/sensors/tof Outdated

Comment thread lib/sensors/tof.hpp

Comment thread lib/sensors/tof Outdated

TomLonergan03 requested changes Nov 9, 2023

View reviewed changes

licornes-fluos changed the title ~~Add files via upload~~ Time-of-flight Sensor Nov 9, 2023

davidbeechey changed the title ~~Time-of-flight Sensor~~ SNS - Time-of-flight Sensor Nov 9, 2023

licornes-fluos marked this pull request as draft November 25, 2023 10:31

sa-fx and others added 12 commits January 25, 2024 19:26

SNS - ToF Fixes (#57)

38d92d0

TimeOfFlight::initialise()

bb9f4e7

Implement TimeOfFlight::getRange()

870d204

16-bit I2C addresses commented out

e1a1434

TimeOfFlight::create() uses initialise()

8f980a2

TimeOfFlight::getRange() public

6651f1b

Added public 16-bit registers to initialise()

97bf3d2

Set constants for known registers

d0078d0

Update comments

ecc2815

TimeOfFlight::getRangeContinuous()

027bf56

Added logs to TimeOfFlight

e9d43a6

Update TODOLater

549cfea

sa-fx reviewed Jan 28, 2024

View reviewed changes

TomLonergan03 requested changes Jan 28, 2024

View reviewed changes

licornes-fluos marked this pull request as ready for review February 1, 2024 18:08

Merge branch 'master' into RheaBose-patch-1

006b989

licornes-fluos added 2 commits February 19, 2024 18:29

array for return addresses

f411a77

debug toml lines

e6b983c

TomLonergan03 reviewed Apr 1, 2024

View reviewed changes

Comment thread lib/sensors/time_of_flight.cpp

fixes

792f914

sa-fx marked this pull request as draft May 2, 2024 08:19

sa-fx requested changes May 2, 2024

View reviewed changes

sa-fx assigned H-Allen and unassigned RheaBose May 2, 2024

H-Allen added 2 commits May 6, 2024 07:46

adding new files

a40394d

.

e77ffe7

sa-fx reviewed May 14, 2024

View reviewed changes

Comment thread lib/sensors/tof.cpp

sa-fx May 14, 2024

Copy link
Copy Markdown

Contributor

Choose a reason for hiding this comment

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

You should delete these tof.cpp/hpp files.

sa-fx reviewed May 14, 2024

View reviewed changes

		@@ -1,4 +1,4 @@
		#include "time_of_flight.hpp"
		#include "tof.hpp"

Uh oh!

Conversation

RheaBose commented Nov 6, 2023 • edited by sa-fx Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DylanCavers left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TomLonergan03 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TomLonergan03 left a comment

Choose a reason for hiding this comment

Uh oh!

sa-fx commented Jan 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TomLonergan03 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sa-fx May 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sa-fx May 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

RheaBose commented Nov 6, 2023 •

edited by sa-fx

Loading

sa-fx commented Jan 28, 2024 •

edited

Loading

sa-fx May 2, 2024 •

edited

Loading

sa-fx May 14, 2024 •

edited

Loading