update takes timestamp

[belm0]

It would be useful for the update methods to accept a timestamp at which the sensor data was recorded, rather than use time.now which assumes a constant latency between sensor reading and calling the update function.

[peterhinch]

This is addressed in the docs (section 3.1). There is variable latency in the time it takes for the MPU9150 magnetometer to stabilise but the coro returns its result with constant latency.

imu = MPU9150('X')  # Instantiate IMU (default orientation)

async def read_coro():
    imu.mag_trigger()  # Hardware dependent: trigger a nonblocking read
    await asyncio.sleep_ms(20)  # Wait for mag to be ready
    return imu.accel.xyz, imu.gyro.xyz, imu.mag_nonblocking.xyz
    # Returned (ax, ay, az), (gx, gy, gz), (mx, my, mz)

fuse = Fusion(read_coro)

This covers cases where an IMU needs time to be configured before it takes a reading. There are other possible sources of latency which could be more problematic. Do you have a particular IMU in mind?

[belm0]

Actually my request is regarding the synchronous API. In my use case timestamped raw IMU data comes over wired LAN. The current synchronous API hard-codes time.now within the update method, so it's not too flexible.

[peterhinch]

Are your timestamps derived from utime.ticks_us()? These roll over at a certain value and are differenced using utime.ticks_diff(). If you're using this standard I'll look at making this change, otherwise you may need to write a custom solution.

[belm0]

I see, yes use of time with arbitrary reference poses a problem for my case within a distributed system.

Perhaps the time difference function itself could be a configuration option. If I end up using this fusion library I may make a pull request along those lines, but otherwise I don't want to ask for refactoring work unless someone will make use of it.

[peterhinch]

You're the first to request it. The change is simple to implement, but any solution in the master branch should be generic.

I think your use case is perhaps rather specialised. Most uses are in drones or robots where the IMU is directly connected to the microcontroller performing the fusion, so latency can be expected to be consistent between readings. You might want to consider your own fork.

I suggest we leave this until you've decided how you will proceed.

[peterhinch]

@turbinenreiter @belm0 I've pushed a branch remote-timestamp with a possible solution for the synchronous version only. Note this is untested for comments.

@turbinenreiter The nub of the issue is the case where the IMU is hosted by a remote device communicating, by a link with variable latency, with the MicroPython host performing fusion. For the fusion algorithm to work the remote should send, along with the vector, a timestamp being the time when the IMU was read. As the remote may not be a MicroPython device, its timestamp may be in a foreign format requiring different scaling and an alternative differencing algorithm.

This proposed solution does not affect the API. A solution to the async version may affect the API in that the coro would need to return a timestamp or None in addition to the vector. (I might be able to code round that with a constructor option).

I do, however, have a doubt over whether this use case is sufficiently common to justify changing master. @turbinenreiter What's your view?

@belm0 I'd be interested to hear your comments or test results.

[turbinenreiter]

In a way this pulls out the time measurement in it's own module, making the fusion code itself more portable.
Doesn't seem like a bad idea to me.

[peterhinch]

@belm0 was not so keen on the DeltaT class. I've pushed an alternative implementation which does the same thing without the class (possibly in both senses of the word :) ).

The new implementation gains simplicity but loses error checking and (arguably) elegance. I'm in two minds which I prefer.

[belm0]

LGTM, thank you

[turbinenreiter]

OK, then let's create a PR with the preferred solution and we can merge after you tested it.

[peterhinch]

Actually I'm changing my mind about this and prefer my original solution for the following reasons:

• I can make the deltat module more OS-agnostic, putting the elapsed_micros function into it. The new version will return a RuntimeError if a user attempts to use the default function on a platform which does not support it.

• All timekeeping is then localised in one module. I think this is a big deal.

• The deltat module will be common to synchronous and async solutions, improving maintainability.

• The constructor call signature can be similar for synchronous and asynchronous solutions. If the async constructor "knows" to expect timestamps I can avoid any API changes in the more common case where they aren't required.

I feel the small loss of code transparency is more than offset by these gains. It's possible that Fusion might even run on CPython with these proposed changes.

@turbinenreiter I think we're a little way off a PR just yet. I want a solution for the async version too, so that we don't have to revisit this. I then need to prove I haven't broken anything. And @belm0 needs to test the timestamp solution in a real system.

[peterhinch]

OK, I've pushed an update. I'm taking the opportunity to address cross-platform issues. Hopefully I've addressed the issue raised by @belm0 in that ensure_ready() is abolished. The class simply returns a dt value, even on the first call. deltat.py has comments about the cross-platform issues: either end of the link may not be running MicroPython.

I think if we're going to go for a solution it needs to be as general as possible.

[peterhinch]

I've done some regression testing on the Pyboard (synchronous and asynchronous versions) and encountered no issues.

@belm0 It would be great if you could test with timestamps.

[peterhinch]

I've pushed an update. This has now been tested with timestamps using data captured from a pyboard. See remote directory, which also contains part-written documentation.

Fusion has been successfully tested in remote mode under CPython (V3.5+ is required for the async version).