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

Issue #189 - Update diff handling of complex types and human readable values #191

Merged
merged 2 commits into from
Jan 25, 2021

Conversation

MJJoyce
Copy link
Member

@MJJoyce MJJoyce commented Jan 21, 2021

Update handling of packet diffing to include complex types (Time, Cmd,
and EVR) in with DN-to-EU values. This pushes human-readable info for
these types to the frontend so clients don't need to inspect the
dictionaries to determine what should be displayed.

Update calls to replace_datetimes so the "dntoeu" diffs are passed
instead of the raw ones. Previously these calls weren't doing anything
since the passed diffs were always raw values. This was likely left in
from a previous iteration on this functionality.

Note, this doesn't make sweeping changes to the diffing code and instead
works around the existing framework. The current stuff is a bit of a
mess and could really use some TLC. We'll need to revisit this in a
follow up ticket.

Update the handling of Field format calls to fix issues encountered with
formatting time data and to handle the new diffed data changes. Format
calls are now always run over the raw data of a field instead the "human
readable" data when dealing with complex types or when the value of the
field is not a "number". This could result from enumerated values on
non-complex field. Manipulations can then be performed as necessary on
the data prior to the format call if the data type requires it. For
example, in the case of a Time object we decode the raw value into a Date
object prior to formatting it so the sprintf library handles it properly.

The dtype definitions for Time32 and Time64 have been updated so they
can be used to decode Date objects from raw values. Note, there's no
clean up done here of the other code in dtype that is effectively dead
given the changes to the front / backend transmission formats. That
needs handled in a follow on ticket.

Note, there is the possibility for semi-strange situations to occur with
packet caching and the current format handling. For example, consider a
command field that is not labelled as raw with values that are undefined
opcodes. The "human readable" packet diff will mark this field as
"Unidentified Cmd" any time the opcode doesn't decode properly. As such,
this field could be incorrectly cached and ignored if the raw value
changed but the "human readable" value didn't since the cache check
occurs on the initial pulled value (which relies on raw = true / false).
Effectively, this means that users may need to be more explicit with
marking fields as raw=true.

Similarly, formatting on a given raw / not value isn't always consistent
since we need to support time value formatting. Unfortunately there
isn't a great way to handle this without significantly reducing
functionality and the caveats (e.g., you can't dump a time field to hex)
aren't likely to cause too much pain and suffering.

Resolve #189
Resolve #190

Update handling of packet diffing to include complex types (Time, Cmd,
and EVR) in with DN-to-EU values. This pushes human-readable info for
these types to the frontend so clients don't need to inspect the
dictionaries to determine what should be displayed.

Update calls to `replace_datetimes` so the "dntoeu" diffs are passed
instead of the raw ones. Previously these calls weren't doing anything
since the passed diffs were always raw values. This was likely left in
from a previous iteration on this functionality.

Note, this doesn't make sweeping changes to the diffing code and instead
works around the existing framework. The current stuff is a bit of a
mess and could really use some TLC. We'll need to revisit this in a
follow up ticket.
Update the handling of Field format calls to fix issues encountered with
formatting time data and to handle the new diffed data changes. Format
calls are now always run over the raw data of a field instead the "human
readable" data when dealing with complex types or when the value of the
field is not a "number". This could result from enumerated values on
non-complex field. Manipulations can then be performed as necessary on
the data prior to the format call if the data type requires it. For
example, in the case of a Time object we decode the raw value into a Date
object prior to formatting it so the sprintf library handles it properly.

The dtype definitions for Time32 and Time64 have been updated so they
can be used to decode Date objects from raw values. Note, there's no
clean up done here of the other code in dtype that is effectively dead
given the changes to the front / backend transmission formats. That
needs handled in a follow on ticket.

Note, there is the possibility for semi-strange situations to occur with
packet caching and the current format handling. For example, consider a
command field that is not labelled as raw with values that are undefined
opcodes. The "human readable" packet diff will mark this field as
"Unidentified Cmd" any time the opcode doesn't decode properly. As such,
this field could be incorrectly cached and ignored if the raw value
changed but the "human readable" value didn't since the cache check
occurs on the initial pulled value (which relies on raw = true / false).
Effectively, this means that users may need to be more explicit with
marking fields as raw=true.

Similarly, formatting on a given raw / not value isn't always consistent
since we need to support time value formatting. Unfortunately there
isn't a great way to handle this without significantly reducing
functionality and the caveats (e.g., you can't dump a time field to hex)
aren't likely to cause too much pain and suffering.
@MJJoyce MJJoyce requested review from a team as code owners January 21, 2021 03:56
@MJJoyce MJJoyce merged commit 10bd4d0 into master Jan 25, 2021
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.

Formatting of Time data fails Backend to Frontend state isn't properly handling complex data types
1 participant