fprime
fprime copied to clipboard
FPP v1.1.0 Integration
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
andSvc/Health/HealthComponentImpl.cpp
- Change unit test cases testing serialization and deserialization of invalid enums in
Svc/ActiveLogger/test/ut/ActiveLoggerImplTester.cpp
andSvc/Health/test/ut/Tester.cpp
to expectFw::CmdResponse::FORMAT_ERROR
instead ofFw::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
toT
- Type
t
kept for backwards compatilibity
- Type
- Add a conversion operator from enum class type to raw enum type
T
:EnumClass :: operator T() const { return this->e; }
-
operator==()
andoperator!=()
functions take in raw enum typeT
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 callsisValid()
and returnsFw::FW_DESERIALIZE_FORMAT_ERROR
ifisValid()
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
inserialize()
/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;
- Primitive members are passed by value, e.g. for a struct with member
- 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];
- E.g. for a struct containing a member
- 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>
- The naming convention for these string classes is
- Format strings for integer types use PRI macros instead of
%u
and%i
@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 xonTo 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.
@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 xonTo 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.
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 this looks good. I will merge once CI is passing above!
@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?
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(
| ^~~~~~~~~~~~~~
It looks like we are getting a duplicate function definition in the header.
This is a known issue. We are working on it. See https://github.com/fprime-community/fpp/issues/186.
@bocchino @tiffany1618 are we ready for a formal v1.1.0?
@LeStarch Yes! I am working on it.
I just kicked off the v1.1.0 release for FPP.
The FPP version is v1.1.0, and CI passed. This is ready for merge!