nasa / fprime-gds

F´ Python Ground Data System (GDS).
https://github.com/nasa/fprime
Apache License 2.0
18 stars 37 forks source link

Add display text if there is no format string #135

Closed SMorettini closed 1 year ago

SMorettini commented 1 year ago
Originating Project/Creator
Affected Component data_types
Affected Architectures(s)
Related Issue(s)
Has Unit Tests (y/n)
Builds Without Errors (y/n)
Unit Tests Pass (y/n)
Documentation Included (y/n)

Change Description

This merge request add a display_text for events having arguments but not a format_string.

Rationale

Before the arguments of an event without a format_string were not logged or printed everywhere. This pull request will enable the visualization of the argument also if the format_string is not set.

An event with no format_string but three arguments(an integer, an enum and a float) will contain in the column Event Description of gds: [{'My_integer_arg': 123}, {'My_enum_arg': 'VALUE_OF_ENUM'}, {'My_float_arg': 12.34}].

bocchino commented 1 year ago

Can you explain the motivation for this PR a bit more?

  1. Event specifiers in FPP are required to have format strings. What is the scenario where an event specifier without a format string is presented to the GDS?
  2. What is the reason for handling just this case of an ill-specified event (n > 0 args and no format specifier)? What about an event that has n args and m replacement fields in its format specifier, for n =/= m? This is also disallowed by FPP.
SMorettini commented 1 year ago

About point 1: I thought about this change because I was using gds to communicate with a subsystem of my project that doesn't use Fprime. So I created the xml dictionary manually and I didn't put the format string. About point 2: I didn't consider that case.

If the idea is to support only the dictionary produced by fpp, then we can close this pull request directly.

LeStarch commented 1 year ago

I see this PR as patching an edge case. What to do when format-string == ""? In FPP the user would have to explicitly place an empty string which likey means that was intended...but that is not useful.

LeStarch commented 1 year ago

Outside of FPP (dictionary) it means the text was hand-edited and the format string was forgotten.