Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create common GSOF library #27058

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Ryanf55
Copy link
Collaborator

@Ryanf55 Ryanf55 commented May 13, 2024

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

@Ryanf55 Ryanf55 added the GPS label May 13, 2024
@Ryanf55 Ryanf55 self-assigned this May 13, 2024
@Ryanf55
Copy link
Collaborator Author

Ryanf55 commented May 17, 2024

Flight tests complete.

@Ryanf55 Ryanf55 force-pushed the 27033-gsof-common-library-for-eahrs branch from 28ef39a to 76f87b5 Compare May 27, 2024 23:05
@Ryanf55 Ryanf55 marked this pull request as ready for review May 27, 2024 23:05
@Ryanf55 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
uint32_t
AP_GSOF::SwapUint32(const uint8_t* src, const uint32_t pos) const
{
uint32_t u;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can just return be32toh_ptr()

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should use be16toh_ptr()

float
AP_GSOF::SwapFloat(const uint8_t* src, const uint32_t pos) const
{
union {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use be32tofloat_ptr()

double
AP_GSOF::SwapDouble(const uint8_t* src, const uint32_t pos) const
{
union {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

be64todouble_ptr()

}

double
AP_GSOF::SwapDouble(const uint8_t* src, const uint32_t pos) const
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or remove the function

@Ryanf55
Copy link
Collaborator Author

Ryanf55 commented May 28, 2024

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
Copy link
Collaborator Author

Ryanf55 commented May 28, 2024

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 Ryanf55 force-pushed the 27033-gsof-common-library-for-eahrs branch from d8771a8 to 8435afc Compare May 29, 2024 02:51
* Add tests too

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

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

Signed-off-by: Ryan Friedman <[email protected]>
@Ryanf55 Ryanf55 force-pushed the 27033-gsof-common-library-for-eahrs branch from 8435afc to 3dec5f3 Compare May 30, 2024 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants