BallAerospace / COSMOS

Ball Aerospace COSMOS
https://ballaerospace.github.io/cosmos-website/
Other
363 stars 129 forks source link

Telemetry Grapher Plot Pop-Up Shows the Wrong X Value #1276

Open joeyahines opened 3 years ago

joeyahines commented 3 years ago

Describe the bug When using the telemetry grapher to plot telemetry, the x-value shown in the point pop-up does not correlate to the correct x-value IF telemetry was not received in a time sequential order. This happens if packets use a derived telemetry item to parse a timestamp from the packet into PACKET_TIME.

To Reproduce Steps to reproduce the behavior:

  1. Open Telemetry Grapher and setup a data object to plot. Hit start.
  2. Using a script in the Script Runner, inject two telemetry packets. The first telemetry packet should have a later timestamp than the 2nd. Example:
    inject_tlm("TARGET", "TELEM", {"TIMESTAMP" =>1617371142.6497962, "VALUE" => 1})
    inject_tlm("TARGET", "TELEM", {"TIMESTAMP" =>1617371132.6497962, "VALUE" => 0})

    The telemetry packet used must have a PACKET_TIME field that is derived from TIMESTAMP

  3. Back on the Telemetry Grapher, hover over the first point. The x value will correspond to the later timestamp but the y-value will correspond to the earlier timestamp.

Expected behavior Pop-ups should show the correct x-value even when packets are sent out of order.

Screenshots Packets sent in time sequential order: image

Packets sent out of time order: image

The overall graph is still right, but the x values in the popups are flipped.

Environment (please complete the following information):

ghost commented 2 years ago

I apologize for how long it's been since this ticket was opened. I think the issue is that by default TlmGrapher uses the PACKET_TIMESECONDS for the x value which is derived from the internal packet_time. I'm actually surprised that the graph was correct. When I try to reproduce I only get a single point. I'm basing my config off the demo:

TELEMETRY INST TELEM BIG_ENDIAN "Test"
  <%= render "_ccsds_tlm.txt", locals: {apid: 6} %>
  APPEND_ITEM TIMESTAMP     32 FLOAT    "timestamp"
  APPEND_ITEM VALUE         32 FLOAT    "value"
  ITEM PACKET_TIME 0 0 DERIVED "Derived time"
    GENERIC_READ_CONVERSION_START
      Time.at(myself.read('TIMESTAMP', :RAW))
    GENERIC_READ_CONVERSION_END

inject_tlm("INST", "TELEM", {"TIMESTAMP" =>1617371142.6497962, "VALUE" => 1}) inject_tlm("INST", "TELEM", {"TIMESTAMP" =>1617371132.6497962, "VALUE" => 0})

TlmGrapher does has the ability to change the Time item used for graphing. If you Edit the Data Object you can change Time Item to PACKET_TIME. However, this didn't work for me because the PACKET_TIME is a Ruby Time object and TlmGrapher is trying to compare floating point numbers. I hacked housekeeping_data_object.rb after line 240 to add:

x_value = x_value.to_f if x_value.is_a? Time

This allowed it to graph. See if those steps correct your issue.

bradley-harden-apl commented 2 years ago

I was about to create an new issue to ask about this topic more generally, but then I found this.

I was going to ask how COSMOS handles time in a general sense, i.e. what defines the time of a given packet. I haven't seen that discussed anywhere in the v5 documentation. However, now that I search for "packet time", I found this blog post.

Is it just a problem of under-documentation in v5? Does the documentation for v4 cover time in more detail? If so, is that information still accurate?

We will also need to receive packets that could be out of order, so setting the packet timestamp via a DERIVED item makes a lot of sense, as long as it works.

bradley-harden-apl commented 2 years ago

As a follow up question, these two statements seem to contradict each other. The blog post link in my previous post says

Definining PACKET_TIME allows the PACKET_TIMESECONDS and PACKET_TIMEFORMATTED to be calculated against an internal Packet time rather than the time that COSMOS receives the packet.

Yet your post @jasonatball says

PACKET_TIMESECONDS ... is always based on the received time

Am I misunderstanding something? If PACKET_TIME is being set correctly as a derived item, then shouldn't PACKET_TIMESECONDS be correctly derived from it? Why would PACKET_TIMESECONDS ever be based on the received time if PACKET_TIME is defined?

jmthomas commented 2 years ago

To answer your first question: Yes the v5 documentation needs a little more love to flesh out topics like time. We are planning to add more of the updated blog posts from v4 to the Guides section of v5.

If a PACKET_TIME item is defined we will use that when calculating the packet time. See packet_time. In general the COSMOS core source code is the same from v4 to v5.

We define all the reserved time items based on conversions which use the packet_time. So my original comment was correct and PACKET_TIMESECONDS is based on an internal packet_time which is calculated by PACKET_TIME or the system clock.

bradley-harden-apl commented 2 years ago

@jmthomas, thanks for the quick reply. I'm still a little confused though.

If PACKET_TIMESECONDS is derived from PACKET_TIME and @joeyahines set PACKET_TIME correctly, then why wasn't the data plotted correctly? Was it just a bug?

From your first post in this issue, it doesn't sound like "just a bug". It sounds more like users need to do something else, in addition to setting PACKET_TIME, for this to work correctly.

jmthomas commented 2 years ago

I think this is indeed a bug in TlmGrapher. It gets a stream of packets from the server and doesn't time order them at all. It simply graphs whatever it gets. Then when we build popups we use login in line_graph_popups.rb which calls lines.rb to get the value. I think this just indexes into the data received rather than by the actual time.

bradley-harden-apl commented 2 years ago

Ok. I tried to find the code in v5, but it looks like things are vastly different. Do you think the same bug exists in v5? I will test out the example above later today.