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

osal Integration candidate: 2021-04-27 #975

Merged
merged 17 commits into from
Apr 29, 2021
Merged

Conversation

astrogeco
Copy link
Contributor

@astrogeco astrogeco commented Apr 28, 2021

Description

PR #972

Fix #970, update documentation for read/write

[docs] Clarifies that that zero will be returned on EOF condition in the API documentation for OS_read/write/TimedRead/TimedWrite. In the case of the timed API calls, the OS_ERR_TIMEOUT status code will be returned if the timeout expired without the handle becoming readable/writable during that time.

PR #966

Fix #832, add "handler" feature to utassert stub API

Addresses a shortcomings in the UT Assert hook functions. Namely the assumed return type of int32 which is not always the case.

Adds the concept of a "handler" function to UT assert to replace hard-coded custom logic in UT assert. A handler is the custom logic that exists between the hook function and the return to the stub caller. The handler is directly responsible for setting all outputs.

Adds a script to auto-generate stub functions that match this pattern. Given an API header file, the script extracts
the declarations, and generates a source file with stub definitions that rely on a separate handler to deal with the needed outputs.

Refactors os-shared-printf.h) into two parts to improve the compatibility with the script method.

Updates all existing stubs in OSAL to use the auto-generated stub logic from the script, created directly from the C header. This ensures that stubs will match the FSW implementation.

Only affects coverage testing with stubs, and should be fully backward compatible with existing tests.

PR #953

Fix #419 - Adds local makefile (trivial single build sample, can use different build directories for multiple platform)
Fix #729 - Adds bundle and local unit test run and coverage verification
Fix #954 - Added missing line coverage

[continuous-integration] Adds a local osal-specific makefile to help build unit tests. Adds a new github workflow that runs the unit tests in both the context of the bundle configuration and the local OSAL config. Verifies 100% line coverage.

PR #971

Fix #969, socket accept using incorrect record
Fixes incorrect token use in OS_SocketAccept. Enables the network-api-test to handle multiple connections that re-use the same acceptor socket between them.

PR #959

Fix #957, move async console option

Promotes the OS_CONFIG_CONSOLE_ASYNC option into the shared layer to remove duplicate implementation code and add more coverage testing.

Adds an osconfig option to allow the user to elect this mode at configuration time.

No externally-visible impacts

Context

Part of nasa/cFS#250

Testing

OSAL Checks https://github.com/nasa/osal/pull/975/checks
cFS bundle Checks https://github.com/nasa/cFS/pull/250/checks

Authors

@jphickey
@skliper

jphickey and others added 12 commits April 12, 2021 11:12
Puts the "async" option into the shared layer instead of the impl layer.
This allows both options to be coverage tested and also allows a bit more
of the logic to be common instead of duplicated in the 3 implementations.

This also adds back an osconfig option to allow the user to elect this
mode at configuration time.
Adds the concept of a "handler" function to UT assert.  A handler
is basically the custom logic that exists between the hook function
and the return to the stub caller.  In current UT stubs, this is
hard coded, and it generally comprises setting output parameters
and translating return values as needed.

This concept adds the basic plumbing to allow the handler to be
configured just like a hook function already does.  The difference
is that the handler is directly responsible for setting all outputs.

This also includes a script to auto-generate stub functions that
match this pattern.  Given an API header file, the script extracts
the declarations, and generates a source file with stub definitions
that rely on a separate handler to deal with the needed outputs.

Note this initial commit only adds the basic framework.  It does
not change any existing stubs or tests, and is fully backward
compatible, as it is a new feature and it is a no-op unless
actually configured/used by the stub or test case.  Follow on
commits will update the stubs to use this pattern.
To imporove scriptability of generating the shared layer stubs, makes
two minor adjustments:

- adds a typedef for the callback in OS_ObjectIdIteratorProcessEntry.  This
  avoids the complex syntax of function pointer argument, which is hard to
  read but also harder to parse in a script, too.
- Splits the "os-shared-printf.h" header into two parts, adding a new
  header file "os-shared-console.h".  This is because OS_ConsoleOutput_Impl
  is implemented in a separate unit in the source, and this allows a better
  relationship between stub files and header files.
The "built-in" logic from existing stubs is converted to a
handler function and moved to a different source file.  The
generated stub references this function if it exists and
uses it as a default handler, so the default behavior is not
changed.

In this pattern the stubs in this directory are strictly
limited to the same public APIs that are defined in header
files under src/os/inc.  This has the side effect of removing
some internal stubs required for coverage test.  Those stubs
will be reinstated in the coverage specific stubs where other
internal functions are.
Update the coverage-specific (shared layer internal) stubs to use
generated stubs.
The OS_SocketAccept call was using the incorrect token, using
the server/acceptor socket when it should have used the connection
socket.

The bug overwrote data in the acceptor socket, but it would only cause
an issue when the user attempted to use the server socket to accept
a second connection, but the tests only performed a single connection.

This also improves the network-api-test to do multiple connections,
re-using the same acceptor socket between them.
Update the API documentation for OS_read/write/TimedRead/TimedWrite.
Clarify that zero will be returned on EOF condition, and in the
case of the timed API calls, the OS_ERR_TIMEOUT status code will
be returned if the timeout expired without the handle becoming
readable/writable during that time.

This is intended to allow the caller to differentiate between a
handle which is in a state where it cannot read/write data (e.g.
at EOF, or a pipe/socket with remote end closed) and handle which
is simply idle or busy.
The stream test was sending an 8-bit count pattern to validate
the connection, but was not using the provided API to do so.

This provides the exact same test but uses the provided API
from UtAssert to implement it, rather than duplicating the logic.

Notably, this also confirms that the tool functions work as
expected, since there were no other OSAL tests using this API.
Fix #970, update documentation for read/write
Fix #832, add "handler" feature to utassert stub API
Fix #419 #729 #954, Adds local makefile and bundle/local unit test actions with coverage verification
Fix #969, socket accept using incorrect record
@astrogeco
Copy link
Contributor Author

@jphickey, @skliper ran into some tests problems at the bundle level

https://github.com/nasa/cFS/actions/runs/792740055

Errors while running CTest
89/89 Test #89: coverage-sample_lib-ALL ............***Failed    0.00 secOutput from these tests are in: /home/runner/work/cFS/cFS/build/native/default_cpu1/Testing/Temporary/LastTest.log

@skliper
Copy link
Contributor

skliper commented Apr 28, 2021 via email

@astrogeco
Copy link
Contributor Author

@skliper It's alive!!!! Thanks!

@astrogeco
Copy link
Contributor Author

astrogeco commented Apr 28, 2021

Looks like my conflict resolution broke this :(

@skliper can you check the unit test workflow?

In file included from /home/runner/work/osal/osal/src/unit-test-coverage/ut-stubs/src/os-shared-console-impl-stubs.c:27:0:
/home/runner/work/osal/osal/src/os/shared/inc/os-shared-console.h:56:3: error: conflicting types for ‘OS_console_internal_record_t’
 } OS_console_internal_record_t;

@skliper
Copy link
Contributor

skliper commented Apr 28, 2021

@astrogeco The OS_console_internal_record_t was moved to src/os/shared/inc/os-shared-console.h. Should remain deleted from the printf.h, but needs the update (add IsSync).

@astrogeco astrogeco marked this pull request as ready for review April 29, 2021 01:56
@astrogeco astrogeco merged commit a061666 into main Apr 29, 2021
astrogeco added a commit to nasa/cFS that referenced this pull request Apr 29, 2021
Combines:

nasa/cFE#1431
nasa/osal#975
nasa/sample_lib#61

Includes:

nasa/cFE#1379, memory pool pointer type
nasa/cFE#1289, ES child task functional test
nasa/cFE#1289, typo in macro name
nasa/cFE#1286, Remove broken BUILDDIR reference
nasa/cFE#1305, remove option for "osal_compatible"
nasa/cFE#1374, CFE_SUCCESS constant type
nasa/cFE#1316, Remove Unused Error Codes
nasa/cFE#1370, better warning about malformed startup line
nasa/cFE#1373, check status of call to `CFE_ES_CDS_CachePreload`
nasa/cFE#1384, update documentation for `CFE_ES_DeleteCDS`
nasa/cFE#1385, exception logic when app/task is not found
nasa/cFE#1372, error if alignment size not a power of two
nasa/cFE#1368, remove unneeded CFE_ES_SYSLOG_APPEND macro
nasa/cFE#1382, improve documentation for resourceID patterns
nasa/cFE#1371, assert `CFE_RESOURCEID_MAX` is a bitmask

nasa/osal#972, update documentation for read/write
nasa/osal#966, add "handler" feature to utassert stub API
nasa/osal#953, Adds local makefile and bundle/local unit test actions with coverage verification
nasa/osal#971, socket accept using incorrect record
nasa/osal#959, move async console option

nasa/sample_lib#60, replace direct ref to ArgPtr with macro
@skliper skliper added this to the 6.0.0 milestone Sep 24, 2021
jphickey pushed a commit to jphickey/osal that referenced this pull request Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment