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

[sources/ni] Added calculated timestamps based on NI box sampling rate to message format #363

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

ChrisYx511
Copy link
Contributor

@ChrisYx511 ChrisYx511 commented Mar 1, 2025

Description

Currently, the NI source sends a batch of datapoints every message, specified by the BULK_READ configuration option, with only one timestamp. This forces the data_processing script to either do weird extrapolation or, in the current implementation, just average the points, reducing the effective sampling rate significantly. This PR adds a new array of relative timestamps to each message, giving each datapoint within the message a distinct timestamp that is computed relatively from the sample rate.

This allows us to expand each message to the actual sample rate rather than aggregating. This also allows it to be more accurate and less jittery than the system timestamp obtained from time.time().

To accompany these changes, an minimum viable product of a new data processing script is included to process the messages with the new timestamp list. The new data format is entirely backwards compatible with old tools and will behave identically with the old data_processing script, dashboard, etc. The new data_processing script allows us to take advantage of the increased accuracy and effective polling rate.

The new data_processing script does not have any fancy CLI or UI right now, does not support CAN, and you have to specify the name of the files in the script itself. Improving this is scheduled but out of scope for this PR.

  • Modified the data format of the NI and fake NI Source as follows:
class DAQ_RECEIVED_MESSAGE_TYPE(TypedDict):
    timestamp: float
    data: dict[str, list[float]]
    """    
    Each sensor groups a certain number of readings, the bulk read rate of the DAQ.
    The length of that list corresponds to the length of relative_timestamps_nanoseconds below.
    The floating point numbers are arbitrary values depending on the unit of the sensor configured when it was recorded.
    """
    # Example: {
    #     "NPT-201: Nitrogen Fill PT (psi)": [1.3, 2.3, 4.3],
    #     "OPT-201: Ox Fill PT (psi)": [2.3, 4.5, 7.2],
    #     ...
    # }
    # 1.3 and 2.3 are the readings for each sensor at t0, 2.3 and 4.5 for t1, etc.

    relative_timestamps_nanoseconds: list[int] # <- this is the new part
    """
    Corresponding timestamps for each reading of every sensors, calculated from sample rate (dt_ns = 1/sample_rate * 10^9).
    There can be variation of +- 1ns for every point, according to NI box data sheet, which is minimal.
    Timestamps are based on initial time t_0 = time.time_ns(), meaning they should be always unique.
    Unit is nanoseconds
    """
    # Example: [19, 22, 25] <- 1.3 and 2.3 from above was read at t0 = 19

    # Rate at which the messages were read, in Hz, dt = 1/sample_rate
    sample_rate: int

    # Arbitrary constant that validates that the received message format is compatible
    # Increment MESSAGE_FORMAT_VERSION both here and in the NI source whenever the structure changes
    message_format_version: int
  • Created new data processing script for DAQ messages to take advantage of the relative_timestamps_nanoseconds list, expanding processed logs back to the original sample rate of the NI source
  • Use time.time_ns() and sample rate for timestamp calculations for better accuracy and no floating point error
  • Enforced type annotation in both sources, as well as strict type checking in new data processing script
  • Other minor changes in formatting

This PR closes #212 .

Developer Testing

Here's what I did to test my changes:

  • Ran NI Source with new format and tested that all old tools (dashboard, old data processing) still work as expected
  • Processed NI source new format with new data processing script and validated timestamp linearity
  • Researched and discussed implication of reading from the NI box with @JasonBrave . Conclusions below.
    • The NI box allocates a buffer of 1000 samples and maintains its own clock to read at exactly the specified sample rate
    • When reading from the buffer, if we're reading faster than the buffer fills up, the call ai.read will wait until it collects enough messages, guaranteeing the interval
    • If we are reading faster, some data points will be overwritten, however that should basically never happen since we are reading as fast as possible with a while True loop, and the sample rate is orders of magnitude smaller than the clock speed of the DAQ computer. The worst case scenario would be the loss of 1 or 2 datapoints, which at a sample rate of 1khz+, shouldn't matter. nidaqmx library should also raise an error in this situation.
    • Computed the imprecision of the calculated timestamp to be roughly +- 1ns, which is perfectly acceptable. We'll say it's +- 10 ns just as a worst case scenario (calcualtion error, python weirdness, etc.). That's still perfectly acceptable when we are talking in orders of milliseconds. (See https://www.ni.com/docs/en-US/bundle/usb-6218-specs/page/specs.html, see Timing Accuracy)

Reviewer Testing

Here's what you should do to quickly validate my changes:

  • Generate a new globallog using fakeni and try parsing using both old and new data_processing scripts
  • Test on actual DAQ computer to make sure it works (use omnibus-TESTING-UNSTABLE-DONOTUSE on DAQ)

This change is Reviewable

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.

Expand out individual DAQ readings for data exports
1 participant