Fixed wrong timing budget

[Cirromulus]

This PR fixes an error in calculation of VCSEL period. Now, if a measurement budget of e.g. 20ms is set, the measurement actually takes (around) 20ms, not 200ms.

Image represents the I2C communication in the top two rows and the interrupt line in the third. Notice the 22ms pulse width instead of 199ms.

[kevin-pololu]

Hi, Cirromulus.

I'm not aware of any issues with the library that cause readings to take 10x longer than they should. Could you post an example program that demonstrates the problem your pull request fixes?

Furthermore, the calcMacroPeriod macro is based directly off of the VL53L0X_calc_macro_period_ps function in the ST API, with the addition of a simple division by 1000 to produce a result in nanoseconds instead of picoseconds (after adding half the denominator, 500, so that the division rounds instead of truncating). Can you explain why you think the original function or its adaptation for this library is wrong?

Kevin

[Cirromulus]

The example program is based on the continuus-example, but sets the Measurement Timing Budget to 20000 (20 ms) before starting. (Do you want the exact program?)

void setup()
{
  Wire.begin();
  pinMode(J0, OUTPUT);
  sensor.setTimeout(500);
  if (!sensor.init())
  {
    Serial.println("Failed to detect and initialize sensor!");
    while (1) {}
  }
  if(!sensor.setMeasurementTimingBudget(20000))
  {
     while(1){}
  }
  sensor.startContinuous();
}

void loop()
{
  digitalWrite(J0, 1);
  uint16_t distance = sensor.readRangeContinuousMillimeters());
  digitalWrite(J0, 0);
  if (sensor.timeoutOccurred()) { Serial.print(" TIMEOUT"); }
  doSomething(distance);
}

Without this modification, the time between the sensor's data ready GPIO pin was ~193ms, causing a noticeable lag in my time-sensitive application. With this modification, the measurement frequency approaches the 50Hz, producing results as expected. To measure this, I used a Saleae Logic analyzer (see previous screenshot. From top to bottom: SDA, SCL, sensor interrupt pin, J0 pin).

Initially I compared this Lib with the original from ST to find the cause, so I know that the calculation there is the same. I don't know the exact root of this calculation. My suggestion is that either the PLL period or the vclks per PLL period is off by a factor of 10. The datasheet does not help very much, but also contradicts the minimum timing budget in the (official) code:

The typical timing budget for a range is 33ms (init/ranging/housekeeping), see Figure 12, with the actual range measurement taking 23ms, see Figure 9. The minimum range measurement period is 8ms. -- DocID029104 Rev 2, Chapter 2.6.2

This supposes an absolute timing budget minimum of 18ms vs 20ms in the code. If the "original" calculations were correct, my sensor should not work with a tenth of the possible timing budget.

Sorry that I can not provide you with the exact root of the miscalculation. I can just suggest that you or someone confirm my measurements.

[kevin-pololu]

I'm not able to reproduce the problem on an ATmega328P (Arduino Uno) or ATmega32U4. When I run your code, I get readings and interrupts separated by less than 20 ms as expected (top is GPIO1 interrupt, bottom is the program's output pin):

However, since you have a pin "J0", I suspect you're using a different controller; what is it? It's possible there's a problem with the library on your specific microcontroller, but I suspect it's somewhere other than the calculation you changed.

[kevin-pololu]

I realized that J0 might just be a macro or constant that you defined somewhere else. Still, it would be good if you could confirm what board you're using, and the full code of your exact program (or a simplified program that still demonstrates the problem) would also be helpful so that I can make sure we're looking at the same thing.

[Cirromulus]

You are right, it is just a constant ("Jumper 0"). I use an AtTiny88 at 5MHz. Thanks for looking into it! I will prepare a simplified program soon.

[Cirromulus]

In my tests with a minimal example, the timing issue did not occur. I am quite unsure about how this static calculation may be affected by interrupts. It even does not depend on any possible timing misconfiguration. If someone else runs into this problem, this PR would be a workaround. Until then, I will try to look into it in my spare time.

[kevin-pololu]

Thanks for the update. As I mentioned before, I don't think there is a problem with the part of the code that you changed; instead, I think there might be an issue somewhere else that your change happened to counteract, so that it gave you roughly the behavior you expected. On the microcontrollers I tested with, the library worked as intended as-is (and actually, when I tried to use it with your changes, it did not work at all).

One thing you might want to look into is whether your ATtiny88 is running out of RAM with your original program, which might cause strange behavior. I'm a little surprised this library works on it at all, and I expect even the library's example program to use up most of the program memory and RAM.

[Cirromulus]

Good point, this may be a thing. The library only fits in release mode, avr reports for the full program (considering that the RAM calculation is more an estimation)

RAM:   [=====     ]  46.3% (used 237 bytes from 512 bytes)
Flash: [========= ]  87.4% (used 7156 bytes from 8192 bytes)

For the minimal working example:

RAM:   [====      ]  44.3% (used 227 bytes from 512 bytes)
Flash: [=======   ]  69.4% (used 5686 bytes from 8192 bytes)

[kevin-pololu]

Closing this for now, but feel free to reopen it if you find more evidence that this is an issue with the library.