fprime icon indicating copy to clipboard operation
fprime copied to clipboard

FPP v1.1.0 Integration

Open tiffany1618 opened this issue 2 years ago • 2 comments

This PR depends on FPP v1.1.0, and cannot be merged until the version is bumped up.

Change Description

An FppTest deployment containing unit tests for the FPP autocoder for enums, arrays, and structs has been added. These unit tests also contain several type-parametrized test suites for enum, array, and string classes that can be re-used in other projects to test C++ code generated by the FPP autocoder. See FppTest/typed_tests/README.md for implementation details.

In addition, there were changes to the enum interface (see below for more details), including the addition of an isValid() function. deserialize() will now call this function automatically, removing the need to perform manual validation checks after deserializing an enum. If the deserialization fails because isValid() returns false, deserialize() will return Fw::FW_DESERIALIZE_FORMAT_ERROR. The following changes have been made to accomodate this new behavior (reviewed 8/18/22):

  • Remove manual enum validation from Svc/ActiveLogger/ActiveLoggerImpl.cpp and Svc/Health/HealthComponentImpl.cpp
  • Change unit test cases testing serialization and deserialization of invalid enums in Svc/ActiveLogger/test/ut/ActiveLoggerImplTester.cpp and Svc/Health/test/ut/Tester.cpp to expect Fw::CmdResponse::FORMAT_ERROR instead of Fw::CmdResponse::VALIDATION_ERROR

Changes to Generated C++ Code

Reviewed 8/16/22.

Enums

  • Add type SerialType for the serial representation type of an enum
  • Rename raw enum type t to T
    • Type t kept for backwards compatilibity
  • Add a conversion operator from enum class type to raw enum type T:
    EnumClass ::
      operator T() const
    {
      return this->e;
    }
    
  • operator==() and operator!=() functions take in raw enum type T instead of enum class type reference:
    • Direct comparison with enum class type still works because of the conversion operator
    //! Equality operator
    bool operator==(
        T e //!< The other enum value
    ) const;
    
    //! Inequality operator
    bool operator!=(
        T e //!< The other enum value
    ) const;
    
  • Add isValid() function
    • Implemented by checking a list of closed intervals computed for each enum, instead of checking each individual enum value
  • deserialize() function calls isValid() and returns Fw::FW_DESERIALIZE_FORMAT_ERROR if isValid() returns false
  • Remove the following assignment operator functions with raw int parameters:
    //! Assignment operator
    EnumClass& operator=(
        const NATIVE_INT_TYPE a //!< The integer to copy
        );
    
    //! Assignment operator
    EnumClass& operator=(
        const NATIVE_UINT_TYPE a //!< The integer to copy
        );
    
  • toString() function optimized
    • Remove unnecessary copying of the current enum value to a local variable
  • Inlined these functions:
    • All constructors
    • operator T()
    • operator==()
    • operator!=()

Arrays

  • Remove TYPE_ID constant

Structs

  • Remove TYPE_ID constant
  • Remove serialization/deserialization of TYPE_ID in serialize()/deserialize()
  • Remove pointer copy constructor
    • Reference copy constructor kept
  • Add non-const getter functions for member arrays and serializable type members
    • Primitive members are passed by value, e.g. for a struct with member mU32: U32:
      //! Get member mU32
      U32 getmU32() const;
      
    • Enum type members are passed by value, e.g. for a struct with member mEnum: EnumClass:
      //! Get member mEnumClass
      EnumClass::T getmEnum() const;
      
    • Member arrays and serializable type members are passed by const or non-const reference, e.g. for a struct with member mStruct: StructClass:
      //! Get member mStruct
      StructClass& getmStruct();
      
      //! Get member mStruct (const)
      const StructClass& getmStruct() const;
      
  • Add types for member arrays to simplify array reference syntax
    • E.g. for a struct containing a member mU32Arr: [3] U32:
      //! The array member types
      typedef U32 Type_of_mU32Arr[3];
      
  • Pass member arrays in getter/setter functions by C++ reference instead of by pointer and size
    • Enforces checking of array length
    • E.g. using the example above:
      //! Get member mU32Arr
      Type_of_mU32Arr& getmU32Arr();
      
      //! Get member mU32Arr (const)
      const Type_of_mU32Arr& getmU32Arr() const;
      
      //! Set member mU32Arr
      void setmU32Arr(const Type_of_mU32Arr& mU32Arr);
      
  • Use initializer lists in constructors instead of calling this->set()
  • Change formatting of member arrays in toString() from [elt] [elt] [elt] to [ elt, elt, elt ]
  • Inline all getter functions

Other

  • Nested string classes for arrays and structs are only generated for unique string sizes
    • The naming convention for these string classes is StringSize<size>
  • Format strings for integer types use PRI macros instead of %u and %i

tiffany1618 avatar Aug 19 '22 17:08 tiffany1618

@check-spelling-bot Report

:red_circle: Please review

See the :open_file_folder: files view or the :scroll:action log for details.

Unrecognized words (5)

getm setm setprecision sstream stringstream

Previously acknowledged words that are now absent BLSPSERIALDRIVERCOMPONENTIMPLCFG getbuffer getsize LINUXUARTDRIVER NDELAY Rce setbuffer xoff xon
To accept :heavy_check_mark: these unrecognized words as correct (and remove the previously acknowledged and now absent words), run the following commands

... in a clone of the [email protected]:tiffany1618/fprime.git repository on the fpp-integration branch (:information_source: how do I use this?):

update_files() {
perl -e '
my @expect_files=qw('".github/actions/spelling/expect.txt"');
@ARGV=@expect_files;
my @stale=qw('"$patch_remove"');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
next if /^(?:$re)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spelling/expect.txt";
use File::Path qw(make_path);
use File::Basename qw(dirname);
make_path (dirname($new_expect_file));
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"$patch_add"');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a)."-".$a cmp lc($b)."-".$b} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;
system("git", "add", $new_expect_file);
'
}

comment_json=$(mktemp)
curl -L -s -S \
-H "Content-Type: application/json" \
"https://api.github.com/repos/nasa/fprime/issues/comments/1220957298" > "$comment_json"
comment_body=$(mktemp)
jq -r ".body // empty" "$comment_json" | tr -d "\\r" > $comment_body
rm $comment_json

patch_remove=$(perl -ne 'next unless s{^</summary>(.*)</details>$}{$1}; print' < "$comment_body")

patch_add=$(perl -e '$/=undef; $_=<>; if (m{Unrecognized words[^<]*</summary>\n*```\n*([^<]*)```\n*</details>$}m) { print "$1" } elsif (m{Unrecognized words[^<]*\n\n((?:\w.*\n)+)\n}m) { print "$1" };' < "$comment_body")

update_files
rm $comment_body
git add -u

Warnings (1)

See the :open_file_folder: files view or the :scroll:action log for details.

:information_source: Warnings Count
:information_source: limited-references 3

See :information_source: Event descriptions for more information.

If the flagged items are false positives

If items relate to a ...

  • binary file (or some other file you wouldn't want to check at all).

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

  • well-formed pattern.

    If you can write a pattern that would match it, try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

github-actions[bot] avatar Aug 19 '22 17:08 github-actions[bot]

@check-spelling-bot Report

:red_circle: Please review

See the :open_file_folder: files view or the :scroll:action log for details.

Unrecognized words (5)

getm setm setprecision sstream stringstream

Previously acknowledged words that are now absent BLSPSERIALDRIVERCOMPONENTIMPLCFG getbuffer getsize LINUXUARTDRIVER NDELAY Rce setbuffer xoff xon
To accept :heavy_check_mark: these unrecognized words as correct (and remove the previously acknowledged and now absent words), run the following commands

... in a clone of the [email protected]:tiffany1618/fprime.git repository on the fpp-integration branch (:information_source: how do I use this?):

update_files() {
perl -e '
my @expect_files=qw('".github/actions/spelling/expect.txt"');
@ARGV=@expect_files;
my @stale=qw('"$patch_remove"');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
next if /^(?:$re)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spelling/expect.txt";
use File::Path qw(make_path);
use File::Basename qw(dirname);
make_path (dirname($new_expect_file));
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"$patch_add"');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a)."-".$a cmp lc($b)."-".$b} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;
system("git", "add", $new_expect_file);
'
}

comment_json=$(mktemp)
curl -L -s -S \
-H "Content-Type: application/json" \
"https://api.github.com/repos/nasa/fprime/issues/comments/1221041966" > "$comment_json"
comment_body=$(mktemp)
jq -r ".body // empty" "$comment_json" | tr -d "\\r" > $comment_body
rm $comment_json

patch_remove=$(perl -ne 'next unless s{^</summary>(.*)</details>$}{$1}; print' < "$comment_body")

patch_add=$(perl -e '$/=undef; $_=<>; if (m{Unrecognized words[^<]*</summary>\n*```\n*([^<]*)```\n*</details>$}m) { print "$1" } elsif (m{Unrecognized words[^<]*\n\n((?:\w.*\n)+)\n}m) { print "$1" };' < "$comment_body")

update_files
rm $comment_body
git add -u

Warnings (1)

See the :open_file_folder: files view or the :scroll:action log for details.

:information_source: Warnings Count
:information_source: limited-references 3

See :information_source: Event descriptions for more information.

If the flagged items are false positives

If items relate to a ...

  • binary file (or some other file you wouldn't want to check at all).

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

  • well-formed pattern.

    If you can write a pattern that would match it, try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

github-actions[bot] avatar Aug 19 '22 19:08 github-actions[bot]

This PR is ready for review, but the CI tests currently won't pass because of an issue with the FPP Graal VM build that is being looked into.

tiffany1618 avatar Oct 27 '22 21:10 tiffany1618

@tiffany1618 this looks good. I will merge once CI is passing above!

LeStarch avatar Nov 04 '22 01:11 LeStarch

@bocchino @tiffany1618 something is going on in the autocoder causing the crash. It seems that something in an autocoder is overloading itself?

Can one of you look into this and let me know if I can assist?

LeStarch avatar Nov 07 '22 17:11 LeStarch

 In file included from /home/runner/work/fprime/fprime/build-fprime-automatic-native/F-Prime/Svc/ComQueue/ComQueueComponentAc.hpp:37,
                 from /home/runner/work/fprime/fprime/Svc/ComQueue/ComQueue.hpp:12,
                 from /home/runner/work/fprime/fprime/Svc/ComQueue/ComQueue.cpp:8:
/home/runner/work/fprime/fprime/build-fprime-automatic-native/F-Prime/Svc/ComQueue/BuffQueueDepthArrayAc.hpp:63:7: error: ‘Svc::BuffQueueDepth::BuffQueueDepth(const ElementType&)’ cannot be overloaded with ‘Svc::BuffQueueDepth::BuffQueueDepth(const ElementType&)’
   63 |       BuffQueueDepth(
      |       ^~~~~~~~~~~~~~
/home/runner/work/fprime/fprime/build-fprime-automatic-native/F-Prime/Svc/ComQueue/BuffQueueDepthArrayAc.hpp:58:7: note: previous declaration ‘Svc::BuffQueueDepth::BuffQueueDepth(const ElementType&)’
   58 |       BuffQueueDepth(
      |       ^~~~~~~~~~~~~~

LeStarch avatar Nov 07 '22 17:11 LeStarch

It looks like we are getting a duplicate function definition in the header.

LeStarch avatar Nov 07 '22 17:11 LeStarch

This is a known issue. We are working on it. See https://github.com/fprime-community/fpp/issues/186.

bocchino avatar Nov 07 '22 17:11 bocchino

@bocchino @tiffany1618 are we ready for a formal v1.1.0?

LeStarch avatar Nov 09 '22 00:11 LeStarch

@LeStarch Yes! I am working on it.

bocchino avatar Nov 09 '22 01:11 bocchino

I just kicked off the v1.1.0 release for FPP.

bocchino avatar Nov 09 '22 01:11 bocchino

The FPP version is v1.1.0, and CI passed. This is ready for merge!

bocchino avatar Nov 09 '22 16:11 bocchino