fprime icon indicating copy to clipboard operation
fprime copied to clipboard

Updates to TlmChan to send multiple channels per ComBuffer

Open timcanham opened this issue 3 years ago • 5 comments

Originating Project/Creator JPL Internal
Affected Component Svc/TlmChan
Affected Architectures(s)
Related Issue(s)
Has Unit Tests (y/n) y
Builds Without Errors (y/n) y
Unit Tests Pass (y/n) y
Documentation Included (y/n)


Change Description

The Svc/TlmChan component will now send as many channel updates as will fit in a Fw::ComBuffer rather than a single channel for each ComBuffer. The multiple implementation files were consolidated into a single TlmChan.cpp. The old unit tests were updated to the new unit test approach.

Rationale

This takes better advantage of the downlink bandwidth by not requiring the overhead of a packet for each channel update.

Testing/Review Recommendations

Tested with unit tests and full system with downlink.

Future Work

None

Note: This requires this PR on the GDS to work correctly: https://github.com/fprime-community/fprime-gds/pull/70

timcanham avatar Jun 23 '22 03:06 timcanham

@check-spelling-bot Report

Unrecognized words, please review:

  • CHANS
  • COMBUFFER
Previously acknowledged words that are now absent aadl Accu adoc alignedallocator asciidoctor asm autodetect autonumbering awt bavail bbd bc bithacks BLSP bootup capout chown cinttypes Classloader classpath compat componentaction compositestructures concat configurator creatingdocsetswithdoxygen deserializer Dinstall DISF dll doall Donatas donsim ecore eps errstr esac fcheck fprim FPRIMEPROTOCOL fstrength getenv gethostbyname getmtime getppid gettempdir gmail Gnd GNDIF groupadd groupmod hdp HOMEPAGE hostent ifchange IFXML IJET includefile inorder INSTALLDIR instanceof interoperability Inttype isfgen isfpluginexec isfxmlwriter itcl javabuilder javac javanature javax jdt jf JFile jmi JOption junit kevensen ldl libgtest magicdraw mcternan mdbasiccomponents mdinternalstructures mdkernel mdports mdprofiles mdxml mdzip memoization memoize MMD mngr mpmcs mscgen nasafprime netdb Netscape's NGAT nh nio nomagic nondetached nroff OMG's OS'es ovrTrace peeker placeholders PLUGINDIR ppid Prepends println propvals PROTOCOLINTERFACE PYPI rcs refman RHEL RPISCHEDCONTEXTS runtest saikiranra seander Simkunas SOCKETIPDRIVERTYPES SQL's sramanan startword strcat strcpy submenu tcl templating textui tkgui tmpd tmpdir toclevels toolbar ubuntu uk ul uml urllib useradd usermod ve virtualenv vmsize vn watney workaround workspaces Xmx Xss Xvfb
Some files were were automatically ignored

These sample patterns would exclude them:

^Drv/BlockDriver/BlockDriver\.hpp$
^Drv/LinuxGpioDriver/LinuxGpioDriver\.hpp$
^Drv/LinuxSpiDriver/LinuxSpiDriver\.hpp$
^Drv/TcpClient/TcpClient\.hpp$
^Fw/Types/Linux/StandardTypes\.hpp$
^Svc/LinuxTime/LinuxTime\.hpp$
^Svc/PrmDb/PrmDb\.hpp$
^requirements\.txt$

You should consider adding them to:

.github/actions/spelling/excludes.txt

File matching is via Perl regular expressions.

To check these files, more of their words need to be in the dictionary than not. You can use patterns.txt to exclude portions, add items to the dictionary (e.g. by adding them to allow.txt), or fix typos.

To accept 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]:timcanham/fprime.git repository on the work/TlmChanMulti2 branch:

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);
'
(cat '.github/actions/spelling/excludes.txt' - <<EOF
$should_exclude_patterns
EOF
) |grep .|
sort -f |
uniq > '.github/actions/spelling/excludes.txt.temp' &&
mv '.github/actions/spelling/excludes.txt.temp' '.github/actions/spelling/excludes.txt'
}

comment_json=$(mktemp)
curl -L -s -S \
  --header "Content-Type: application/json" \
  "https://api.github.com/repos/nasa/fprime/issues/comments/1163875236" > "$comment_json"
comment_body=$(mktemp)
jq -r .body < "$comment_json" > $comment_body
rm $comment_json

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

patch_add=$(perl -e '$/=undef;
$_=<>;
s{<details>.*}{}s;
s{^#.*}{};
s{\n##.*}{};
s{(?:^|\n)\s*\*}{}g;
s{\s+}{ }g;
print' < "$comment_body")
  

should_exclude_patterns=$(perl -e '$/=undef;
$_=<>;
exit unless s{(?:You should consider excluding directory paths|You should consider adding them to).*}{}s;
s{.*These sample patterns would exclude them:}{}s;
s{.*\`\`\`([^`]*)\`\`\`.*}{$1}m;
print' < "$comment_body" | grep . || true)

update_files
rm $comment_body
git add -u

github-actions[bot] avatar Jun 23 '22 03:06 github-actions[bot]

@check-spelling-bot Report

Unrecognized words, please review:

  • CHANS
  • COMBUFFER
Previously acknowledged words that are now absent aadl Accu adoc alignedallocator asciidoctor asm autodetect autonumbering awt bavail bbd bc bithacks BLSP bootup capout chown cinttypes Classloader classpath compat componentaction compositestructures concat configurator creatingdocsetswithdoxygen deserializer Dinstall DISF dll doall Donatas donsim ecore eps errstr esac fcheck fprim FPRIMEPROTOCOL fstrength getenv gethostbyname getmtime getppid gettempdir gmail Gnd GNDIF groupadd groupmod hdp HOMEPAGE hostent ifchange IFXML IJET includefile inorder INSTALLDIR instanceof interoperability Inttype isfgen isfpluginexec isfxmlwriter itcl javabuilder javac javanature javax jdt jf JFile jmi JOption junit kevensen ldl libgtest magicdraw mcternan mdbasiccomponents mdinternalstructures mdkernel mdports mdprofiles mdxml mdzip memoization memoize MMD mngr mpmcs mscgen nasafprime netdb Netscape's NGAT nh nio nomagic nondetached nroff OMG's OS'es ovrTrace peeker placeholders PLUGINDIR ppid Prepends println propvals PROTOCOLINTERFACE PYPI rcs refman RHEL RPISCHEDCONTEXTS runtest saikiranra seander Simkunas SOCKETIPDRIVERTYPES SQL's sramanan startword strcat strcpy submenu tcl templating textui tkgui tmpd tmpdir toclevels toolbar ubuntu uk ul uml urllib useradd usermod ve virtualenv vmsize vn watney workaround workspaces Xmx Xss Xvfb
Some files were were automatically ignored

These sample patterns would exclude them:

^Drv/BlockDriver/BlockDriver\.hpp$
^Drv/LinuxGpioDriver/LinuxGpioDriver\.hpp$
^Drv/LinuxSpiDriver/LinuxSpiDriver\.hpp$
^Drv/TcpClient/TcpClient\.hpp$
^Fw/Types/Linux/StandardTypes\.hpp$
^Svc/LinuxTime/LinuxTime\.hpp$
^Svc/PrmDb/PrmDb\.hpp$
^requirements\.txt$

You should consider adding them to:

.github/actions/spelling/excludes.txt

File matching is via Perl regular expressions.

To check these files, more of their words need to be in the dictionary than not. You can use patterns.txt to exclude portions, add items to the dictionary (e.g. by adding them to allow.txt), or fix typos.

To accept 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]:timcanham/fprime.git repository on the work/TlmChanMulti2 branch:

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);
'
(cat '.github/actions/spelling/excludes.txt' - <<EOF
$should_exclude_patterns
EOF
) |grep .|
sort -f |
uniq > '.github/actions/spelling/excludes.txt.temp' &&
mv '.github/actions/spelling/excludes.txt.temp' '.github/actions/spelling/excludes.txt'
}

comment_json=$(mktemp)
curl -L -s -S \
  --header "Content-Type: application/json" \
  "https://api.github.com/repos/nasa/fprime/issues/comments/1163876768" > "$comment_json"
comment_body=$(mktemp)
jq -r .body < "$comment_json" > $comment_body
rm $comment_json

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

patch_add=$(perl -e '$/=undef;
$_=<>;
s{<details>.*}{}s;
s{^#.*}{};
s{\n##.*}{};
s{(?:^|\n)\s*\*}{}g;
s{\s+}{ }g;
print' < "$comment_body")
  

should_exclude_patterns=$(perl -e '$/=undef;
$_=<>;
exit unless s{(?:You should consider excluding directory paths|You should consider adding them to).*}{}s;
s{.*These sample patterns would exclude them:}{}s;
s{.*\`\`\`([^`]*)\`\`\`.*}{$1}m;
print' < "$comment_body" | grep . || true)

update_files
rm $comment_body
git add -u

github-actions[bot] avatar Jun 23 '22 03:06 github-actions[bot]

This pull request introduces 1 alert when merging 04f4a499027f3ebda7649c6f093c755ade89b65c into 34c6fb2de608c73c3a514d8658138bbe6e872a20 - view on LGTM.com

new alerts:

  • 1 for Empty branch of conditional

lgtm-com[bot] avatar Jun 23 '22 03:06 lgtm-com[bot]

This pull request introduces 1 alert when merging 3348e692028bc19e05425fc880fd216033f8c901 into 76ee2e3d58f2099f2f065ed626c009dd18136dad - view on LGTM.com

new alerts:

  • 1 for Empty branch of conditional

lgtm-com[bot] avatar Jun 24 '22 04:06 lgtm-com[bot]

This pull request introduces 1 alert when merging dfd7616d982618016ef6fa359a871562af2bef13 into 76ee2e3d58f2099f2f065ed626c009dd18136dad - view on LGTM.com

new alerts:

  • 1 for Empty branch of conditional

lgtm-com[bot] avatar Jun 27 '22 04:06 lgtm-com[bot]

This pull request introduces 1 alert when merging d5501972e8ffcf0b38af2878743ac4feb0df0845 into 32086326ca01af2515b1f8a3c533c86cea029c7f - view on LGTM.com

new alerts:

  • 1 for Empty branch of conditional

lgtm-com[bot] avatar Oct 20 '22 18:10 lgtm-com[bot]

This will pass once we are able to publish our python package. It is blocked due to this issue: https://github.com/pypa/pypi-support/issues/2317. See: https://github.com/fprime-community/fprime-gds/actions/runs/3292405195

LeStarch avatar Oct 20 '22 20:10 LeStarch

@timcanham It appears some merge conflicts crept in and need to be fixed.

bocchino avatar Jan 06 '23 17:01 bocchino

@bocchino Fixed the conflicts. Ready to check again.

timcanham avatar Jan 10 '23 01:01 timcanham

@timcanham some questions here:

  1. static analysis is reporting a bunch of new violations. Most are irrelevant, but the "check return value" items are likely apt. Even if we just need to assert on them
  2. I see a lot of commented-out-code

Thoughts?

LeStarch avatar Jan 12 '23 23:01 LeStarch

@LeStarch Is the "commented out code" you are referring to the code in the unit test?

timcanham avatar Jan 12 '23 23:01 timcanham

@timcanham yes, I had intended to say "commented out code in the UTs". Is this code necessary anymore? If so, should we enable it, if not should we delete it?

LeStarch avatar Jan 13 '23 00:01 LeStarch