Create common GSOF library #27058

Ryanf55 · 2024-05-13T23:07:32Z

Purpose

Factor out the parser into its own library so we can use it in EARHS for the PX1. Do it with as little change to the code as possible.

Testing Done

SITL test flights in a few different modes on copter and plane. I've also tested on the bench hardware connected to SITL. and a flight test.

What changed

95% of the code is the same as before, the only difference is that the number of expected packets (the valid count), is now exposed up to higher levels. The parser used to hard-code 5 different packet types, which breaks down in EARHS.
Otherwise, the parser no longer depends directly on types in AP_GPS, so there are now a function in AP_GPS_GSOF called pack_state_data to go from the packed GSOF data into the AP_GPS struct.

Longer term, I'd like to switch to a bitfield on which packets were received per DCOL frame.

Next up

Add the EARHS. To see the WIP branch, see here: https://github.com/Ryanf55/ardupilot/tree/27033-gsof-add-eahrs-driver

Issue

Relates to AP_GPS_GSOF: Initial EAHRS support #27033

Ryanf55 · 2024-05-17T20:33:00Z

Flight tests complete.

libraries/AP_GPS/AP_GPS_GSOF.cpp

libraries/AP_GSOF/AP_GSOF.cpp

tridge · 2024-05-27T23:59:52Z

libraries/AP_GSOF/AP_GSOF.cpp

+uint32_t
+AP_GSOF::SwapUint32(const uint8_t* src, const uint32_t pos) const
+{
+    uint32_t u;


can just return be32toh_ptr()

If it's ok, I plan to convert all these in a follow up PR. This was moving the code to a library in an attempt not to change anything.

tridge · 2024-05-28T00:00:11Z

libraries/AP_GSOF/AP_GSOF.cpp

+AP_GSOF::SwapUint16(const uint8_t* src, const uint32_t pos) const
+{
+    uint16_t u;
+    memcpy(&u, &src[pos], sizeof(u));


should use be16toh_ptr()

tridge · 2024-05-28T00:00:49Z

libraries/AP_GSOF/AP_GSOF.cpp

+float
+AP_GSOF::SwapFloat(const uint8_t* src, const uint32_t pos) const
+{
+    union {


please use be32tofloat_ptr()

tridge · 2024-05-28T00:01:01Z

libraries/AP_GSOF/AP_GSOF.cpp

+double
+AP_GSOF::SwapDouble(const uint8_t* src, const uint32_t pos) const
+{
+    union {


be64todouble_ptr()

rmackay9 · 2024-05-28T00:02:36Z

libraries/AP_GSOF/AP_GSOF.cpp

+}
+
+double
+AP_GSOF::SwapDouble(const uint8_t* src, const uint32_t pos) const


I know it's existing code but we normally done use CamelCase... or maybe it's Pascal case.

or remove the function

Ditto above

libraries/AP_GSOF/AP_GSOF.h

Ryanf55 · 2024-05-28T00:08:55Z

Valgrind results:

$ valgrind --soname-synonyms=somalloc=nouserintercepts ./build/linux/tests/test_gsof 
==46624== Memcheck, a memory error detector
==46624== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==46624== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==46624== Command: ./build/linux/tests/test_gsof
==46624== 
[==========] Running 2 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 2 tests from AP_GSOF
[ RUN      ] AP_GSOF.incomplete_packet
[       OK ] AP_GSOF.incomplete_packet (5 ms)
[ RUN      ] AP_GSOF.packet1
[       OK ] AP_GSOF.packet1 (9 ms)
[----------] 2 tests from AP_GSOF (17 ms total)

[----------] Global test environment tear-down
[==========] 2 tests from 1 test suite ran. (39 ms total)
[  PASSED  ] 2 tests.
==46624== 
==46624== HEAP SUMMARY:
==46624==     in use at exit: 0 bytes in 0 blocks
==46624==   total heap usage: 222 allocs, 222 frees, 118,093 bytes allocated
==46624== 
==46624== All heap blocks were freed -- no leaks are possible
==46624== 
==46624== For lists of detected and suppressed errors, rerun with: -s
==46624== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Ryanf55 · 2024-05-28T00:32:39Z

Valgrind doesn't work on the autotest right now:

 ./Tools/autotest/autotest.py build.Copter  test.CopterTests2b.GPSTypes --valgrind
 ...
 arducopter: valgrind: Startup or configuration error:
arducopter: valgrind:    Can't create client cmdline file in /home/ryan/Dev/ardu_ws/src/ardupilot/tmp/valgrind_proc_70431_cmdline_471259db
arducopter: valgrind: Unable to start up properly.  Giving up.
>>>> FAILED STEP: test.CopterTests2b.GPSTypes at Mon May 27 18:29:49 2024 (End Of File (EOF). Exception style platform.
<pexpect.pty_spawn.spawn object at 0x7f23d9dcdd20>
command: /usr/bin/valgrind
args: [b'/usr/bin/valgrind', b'--soname-synonyms=somalloc=nouserintercepts', b'--vgdb-prefix=/tmp/vgdb-pipe', b'-q', b'--log-file=arducopter-+-valgrind.log', b'/home/ryan/Dev/ardu_ws/src/ardupilot/build/sitl/bin/arducopter', b'-w', b'-S', b'--home', b'-35.362938,149.165085,584,270', b'--model', b'+', b'--serial1=tcp:2', b'--defaults', b'/tmp/tmpjmreei1d,/home/ryan/Dev/ardu_ws/src/ardupilot/Tools/autotest/default_params/copter.parm']
buffer (last 100 chars): ''
before (last 100 chars): 'pilot/tmp/valgrind_proc_70431_cmdline_471259db\r\nvalgrind: Unable to start up properly.  Giving up.\r\n'
after: <class 'pexpect.exceptions.EOF'>
match: None
match_index: None
exitstatus: None
flag_eof: True
pid: 70431
child_fd: 6
closed: False
timeout: 5
delimiter: <class 'pexpect.exceptions.EOF'>
logfile: <pysim.util.PSpawnStdPrettyPrinter object at 0x7f23d9dce080>
logfile_read: None
logfile_send: None
maxread: 2000
ignorecase: False
searchwindowsize: None
delaybeforesend: 0.05
delayafterclose: 0.1
delayafterterminate: 0.1
searcher: searcher_re:
    0: re.compile('Waiting for '))
Traceback (most recent call last):
  File "/home/ryan/Dev/ardu_ws/src/ardupilot/./Tools/autotest/autotest.py", line 719, in run_tests
    success = run_step(step)
  File "/home/ryan/Dev/ardu_ws/src/ardupilot/./Tools/autotest/autotest.py", line 526, in run_step
    return run_specific_test(specific_test_to_run, binary, **fly_opts)
  File "/home/ryan/Dev/ardu_ws/src/ardupilot/./Tools/autotest/autotest.py", line 387, in run_specific_test
    return tester.autotest(tests=[a], allow_skips=False, step_name=step), tester
  File "/home/ryan/Dev/ardu_ws/src/ardupilot/Tools/autotest/vehicle_test_suite.py", line 14091, in autotest
    results = self.run_tests(tests)
  File "/home/ryan/Dev/ardu_ws/src/ardupilot/Tools/autotest/vehicle_test_suite.py", line 11780, in run_tests
    self.init()
  File "/home/ryan/Dev/ardu_ws/src/ardupilot/Tools/autotest/vehicle_test_suite.py", line 8738, in init
    self.start_SITL()
  File "/home/ryan/Dev/ardu_ws/src/ardupilot/Tools/autotest/vehicle_test_suite.py", line 8652, in start_SITL
    self.sitl = util.start_SITL(binary, **start_sitl_args)
  File "/home/ryan/Dev/ardu_ws/src/ardupilot/Tools/autotest/pysim/util.py", line 608, in start_SITL
    child.expect('Waiting for ', timeout=300)
  File "/home/ryan/.local/lib/python3.10/site-packages/pexpect/spawnbase.py", line 354, in expect
    return self.expect_list(compiled_pattern_list,
  File "/home/ryan/.local/lib/python3.10/site-packages/pexpect/spawnbase.py", line 383, in expect_list
    return exp.expect_loop(timeout)
  File "/home/ryan/.local/lib/python3.10/site-packages/pexpect/expect.py", line 179, in expect_loop
    return self.eof(e)
  File "/home/ryan/.local/lib/python3.10/site-packages/pexpect/expect.py", line 122, in eof
    raise exc
pexpect.exceptions.EOF: End Of File (EOF). Exception style platform.
<pexpect.pty_spawn.spawn object at 0x7f23d9dcdd20>
command: /usr/bin/valgrind
args: [b'/usr/bin/valgrind', b'--soname-synonyms=somalloc=nouserintercepts', b'--vgdb-prefix=/tmp/vgdb-pipe', b'-q', b'--log-file=arducopter-+-valgrind.log', b'/home/ryan/Dev/ardu_ws/src/ardupilot/build/sitl/bin/arducopter', b'-w', b'-S', b'--home', b'-35.362938,149.165085,584,270', b'--model', b'+', b'--serial1=tcp:2', b'--defaults', b'/tmp/tmpjmreei1d,/home/ryan/Dev/ardu_ws/src/ardupilot/Tools/autotest/default_params/copter.parm']
buffer (last 100 chars): ''
before (last 100 chars): 'pilot/tmp/valgrind_proc_70431_cmdline_471259db\r\nvalgrind: Unable to start up properly.  Giving up.\r\n'
after: <class 'pexpect.exceptions.EOF'>
match: None
match_index: None
exitstatus: None
flag_eof: True
pid: 70431
child_fd: 6
closed: False
timeout: 5
delimiter: <class 'pexpect.exceptions.EOF'>
logfile: <pysim.util.PSpawnStdPrettyPrinter object at 0x7f23d9dce080>
logfile_read: None
logfile_send: None
maxread: 2000
ignorecase: False
searchwindowsize: None
delaybeforesend: 0.05
delayafterclose: 0.1
delayafterterminate: 0.1
searcher: searcher_re:
    0: re.compile('Waiting for ')
FAILED 1 tests: ['test.CopterTests2b.GPSTypes']

Ryanf55 · 2024-06-12T21:20:47Z

@peterbarker I am still facing the valgrind problem after rebasing. Can you try running the test on your system?

Ryanf55 · 2024-07-25T14:18:35Z

I just rebased on master, the valgrind test doesn't work still. Considering this was already flight tested successfully, can we merge it? AP_GSOF doesn't call new.

Ryanf55 · 2024-07-26T17:14:16Z

Need to fix CI:

    OUT: [----------] 2 tests from AP_GSOF
    OUT: [ RUN      ] AP_GSOF.incomplete_packet
    OUT: [       OK ] AP_GSOF.incomplete_packet (0 ms)
    OUT: [ RUN      ] AP_GSOF.packet1
    OUT: ../../libraries/AP_GSOF/tests/test_gsof.cpp:25: Failure
    OUT: Expected: (fp) != (nullptr), actual: NULL vs (nullptr)
check: 1 of 49 tests failed
    /__w/ardupilot/ardupilot/build/linux/tests/test_gsof

Ryanf55 · 2024-07-31T02:49:33Z

Need to fix CI:

    OUT: [----------] 2 tests from AP_GSOF
    OUT: [ RUN      ] AP_GSOF.incomplete_packet
    OUT: [       OK ] AP_GSOF.incomplete_packet (0 ms)
    OUT: [ RUN      ] AP_GSOF.packet1
    OUT: ../../libraries/AP_GSOF/tests/test_gsof.cpp:25: Failure
    OUT: Expected: (fp) != (nullptr), actual: NULL vs (nullptr)
check: 1 of 49 tests failed
    /__w/ardupilot/ardupilot/build/linux/tests/test_gsof

This works:

waf tests
build/sitl/tests/test_gsof

This doesn't.

CI_BUILD_TARGET="linux" ./Tools/scripts/build_ci.sh

Until we have a better option for loading in a data file, I'm disabling the binary file test.

tridge · 2024-08-03T00:07:35Z

@Ryanf55 where are we up to with this?

Ryanf55 · 2024-08-04T23:06:39Z

@Ryanf55 where are we up to with this?

I've met the requests of last dev call from Randy. The changes of functions to use the functions in spare-endian is ready for a follow PR.

This has been flown and has unit tests.

Therefore, I would like to request a merge.

will handle in followup PR

* Add tests too Signed-off-by: Ryan Friedman <[email protected]>

Signed-off-by: Ryan Friedman <[email protected]>

Ryanf55 added the GPS label May 13, 2024

Ryanf55 self-assigned this May 13, 2024

Ryanf55 added NeedsTesting and removed NeedsTesting labels May 13, 2024

Ryanf55 added the DevCallTopic label May 27, 2024

Ryanf55 force-pushed the 27033-gsof-common-library-for-eahrs branch from 28ef39a to 76f87b5 Compare May 27, 2024 23:05

Ryanf55 marked this pull request as ready for review May 27, 2024 23:05

Ryanf55 force-pushed the 27033-gsof-common-library-for-eahrs branch 2 times, most recently from 5abfae8 to d8771a8 Compare May 27, 2024 23:34

CraigElder removed the DevCallTopic label May 27, 2024

rmackay9 reviewed May 27, 2024

View reviewed changes

libraries/AP_GPS/AP_GPS_GSOF.cpp Outdated Show resolved Hide resolved

rmackay9 reviewed May 28, 2024

View reviewed changes

libraries/AP_GPS/AP_GPS_GSOF.cpp Outdated Show resolved Hide resolved

rmackay9 reviewed May 28, 2024

View reviewed changes

libraries/AP_GSOF/AP_GSOF.cpp Outdated Show resolved Hide resolved

tridge previously requested changes May 28, 2024

View reviewed changes

rmackay9 reviewed May 28, 2024

View reviewed changes

libraries/AP_GSOF/AP_GSOF.h Outdated Show resolved Hide resolved

Ryanf55 force-pushed the 27033-gsof-common-library-for-eahrs branch 2 times, most recently from 8435afc to 3dec5f3 Compare May 30, 2024 15:51

Ryanf55 force-pushed the 27033-gsof-common-library-for-eahrs branch from 3dec5f3 to 59e078a Compare July 19, 2024 04:27

AP_GPS: factor out common library for GSOF

6862b50

* Add tests too Signed-off-by: Ryan Friedman <[email protected]>

Ryanf55 added 4 commits August 19, 2024 08:12

AP_GSOF: factor out common library for GSOF

1d763ac

* Add tests too Signed-off-by: Ryan Friedman <[email protected]>

Tools: factor out common library for GSOF

34f1607

* Add tests too Signed-off-by: Ryan Friedman <[email protected]>

Tools: AP_Periph: Add AP_GSOF to libraries

4a87267

Signed-off-by: Ryan Friedman <[email protected]>

AP_GSOF: Disable packet1 test for now

8af24af

Signed-off-by: Ryan Friedman <[email protected]>

tridge force-pushed the 27033-gsof-common-library-for-eahrs branch from c25b6c9 to 8af24af Compare August 18, 2024 22:12

tridge added the MergeOnCIPass label Aug 18, 2024

tridge approved these changes Aug 18, 2024

View reviewed changes

tridge merged commit d859e9a into ArduPilot:master Aug 20, 2024
93 checks passed

Ryanf55 mentioned this pull request Aug 21, 2024

AP_GSOF: Use sparse endian instead of custom code #27887

Merged

Ryanf55 deleted the 27033-gsof-common-library-for-eahrs branch September 10, 2024 04:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create common GSOF library #27058

Create common GSOF library #27058

Ryanf55 commented May 13, 2024 •

edited

Loading

Ryanf55 commented May 17, 2024

tridge May 27, 2024

Ryanf55 Jul 25, 2024

tridge May 28, 2024

tridge May 28, 2024

tridge May 28, 2024

rmackay9 May 28, 2024

tridge May 28, 2024

Ryanf55 Jul 25, 2024

Ryanf55 commented May 28, 2024

Ryanf55 commented May 28, 2024

Ryanf55 commented Jun 12, 2024

Ryanf55 commented Jul 25, 2024

Ryanf55 commented Jul 26, 2024

Ryanf55 commented Jul 31, 2024

tridge commented Aug 3, 2024

Ryanf55 commented Aug 4, 2024

Create common GSOF library #27058

Create common GSOF library #27058

Conversation

Ryanf55 commented May 13, 2024 • edited Loading

Purpose

Testing Done

What changed

Next up

Issue

Ryanf55 commented May 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ryanf55 commented May 28, 2024

Ryanf55 commented May 28, 2024

Ryanf55 commented Jun 12, 2024

Ryanf55 commented Jul 25, 2024

Ryanf55 commented Jul 26, 2024

Ryanf55 commented Jul 31, 2024

tridge commented Aug 3, 2024

Ryanf55 commented Aug 4, 2024

Ryanf55 commented May 13, 2024 •

edited

Loading