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.

Sign up for GitHub

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

Jump to bottom

logging: backend: uart: improve compatibility, increase test coverage and minor ROM optimizations #65150

Merged
merged 4 commits into from
Nov 14, 2023
Merged

logging: backend: uart: improve compatibility, increase test coverage and minor ROM optimizations #65150

merged 4 commits into from
Nov 14, 2023

Conversation

ycsin
Copy link
Member

@ycsin ycsin commented Nov 14, 2023 •
edited
Loading

  • Fixes CI and improves the UART log backend compatibility with out-of-tree applications post logging: uart: support multiple instances #64917.
  • Increases test coverage of subsys/logging/log_backend_uart.
  • Optimized log_backend_uart to save 1 pointer size in ROM.
  • Changes struct device *device to struct device *uart_dev

fixes #65151

__

(.venv) ycsin@LAPTOP-ROG:~/zephyrproject$ twister -i -T zephyr/tests/subsys/logging/log_backend_uart/ -T zephyr/tests/subsys/logging/log_core_additional/
INFO    - Using Ninja..
INFO    - Zephyr version: zephyr-v3.5.0-1408-gfdff4ea27362
INFO    - Using 'zephyr' toolchain.
INFO    - Selecting default platforms per test case
INFO    - Building initial testsuite list...
INFO    - Writing JSON report /home/ycsin/zephyrproject/twister-out/testplan.json
INFO    - JOBS: 8
INFO    - Adding tasks to the queue...
INFO    - Added initial list of jobs to queue
INFO    - Total complete:  205/ 205  100%  skipped:  103, failed:    0, error:    0
INFO    - 5 test scenarios (205 test instances) selected, 103 configurations skipped (97 by static filter, 6 at runtime).
INFO    - 102 of 205 test configurations passed (100.00%), 0 failed, 0 errored, 103 skipped with 0 warnings in 865.56 seconds
INFO    - In total 480 test cases were executed, 1246 skipped on 41 out of total 639 platforms (6.42%)
INFO    - 102 test configurations executed on platforms, 0 test configurations were only built.
INFO    - Saving reports...
INFO    - Writing JSON report /home/ycsin/zephyrproject/twister-out/twister.json
INFO    - Writing xunit report /home/ycsin/zephyrproject/twister-out/twister.xml...
INFO    - Writing xunit report /home/ycsin/zephyrproject/twister-out/twister_report.xml...
INFO    - Run completed

@ycsin ycsin requested a review from nordic-krch as a code owner November 14, 2023 03:13
@zephyrbot zephyrbot requested a review from dcpleung November 14, 2023 03:14
@ycsin ycsin marked this pull request as draft November 14, 2023 03:20
@ycsin
logging: backend: uart: append index conditionally for compatibility
fdff4ea 
Updated the `LBU_DEFINE` so that the index is appended only
when given to improve backward compatibility.

When it is depending on the `zephyr,console` node, the backend
is defined as `log_backend_uart`.

When it is depending on the new `zephyr,log-uart` node, the
backend is defined as `log_backend_uart0`,
`log_backend_uart1`, and so on.

Updated the names of the internal variables to follow the same
naming convention.

Signed-off-by: Yong Cong Sin <[email protected]>
@ycsin ycsin requested review from cfriedt and nordicjm November 14, 2023 03:23
@ycsin ycsin added bug The issue is a bug, or the PR is fixing a bug Regression Something, which was working, does not anymore Hotfix Fix for issues blocking development, i.e. CI issues, tests failing in CI, etc. Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc. labels Nov 14, 2023
@ycsin
tests: logging: uart: increase test coverage
d1f1a0d 
The test should work on any boards with UART console, so
increase the test coverage by adding `CONFIG_UART_CONSOLE`
filter and using `qemu_x86` as `integration_platforms` instead
of `platform_allow`.

Signed-off-by: Yong Cong Sin <[email protected]>
@ycsin ycsin marked this pull request as ready for review November 14, 2023 05:18
@ycsin ycsin requested a review from nashif as a code owner November 14, 2023 05:18
@ycsin ycsin changed the title logging: backend: uart: append index conditionally for compatibility logging: backend: uart: append index conditionally for better compatibility Nov 14, 2023
ycsin added 2 commits November 14, 2023 14:21
@ycsin
logging: backend: uart: variable shouldn't have the same name as struct
b3dd4b1 
Rename the `device` variable in the `struct lbu_cb_ctx` to
`uart_dev`, as it is a convention in Zephyr to not have the
struct variable name after the struct.

Signed-off-by: Yong Cong Sin <[email protected]>
@ycsin
logging: backend: uart: compile the uart_dev pointer conditionally
89b3b5a 
Compile the `uart_dev` pointer only when necessary
(when `zephyr,log-uart` is used), this saves 4 bytes in
32-bit systems and 8 bytes in 64-bit systems.

Signed-off-by: Yong Cong Sin <[email protected]>
@ycsin ycsin mentioned this pull request Nov 14, 2023
Closed
@ycsin ycsin changed the title logging: backend: uart: append index conditionally for better compatibility logging: backend: uart: improve compatibility, increase test coverage and minor ROM optimizations Nov 14, 2023
@ycsin
Copy link
Member Author

ycsin commented Nov 14, 2023

Both log_core_additional and log_backend_uart tests triggered and ran successfully in the CI

@ycsin ycsin requested a review from jgl-meta November 14, 2023 08:03
@ycsin ycsin requested a review from carlescufi November 14, 2023 17:03
@nashif nashif merged commit aa71ed4 into zephyrproject-rtos:main Nov 14, 2023
20 checks passed
@ycsin ycsin deleted the pr/lbu_idx branch November 15, 2023 01:29
@nordic-krch nordic-krch mentioned this pull request Nov 15, 2023
Closed
@henrikbrixandersen henrikbrixandersen mentioned this pull request Nov 21, 2023
Closed
@ycsin ycsin restored the pr/lbu_idx branch November 21, 2023 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nordicjm nordicjm nordicjm approved these changes

@nordic-krch nordic-krch nordic-krch approved these changes

@jhedberg jhedberg jhedberg approved these changes

@cfriedt cfriedt cfriedt approved these changes

@dcpleung dcpleung Awaiting requested review from dcpleung

@nashif nashif Awaiting requested review from nashif

@jgl-meta jgl-meta Awaiting requested review from jgl-meta

@carlescufi carlescufi Awaiting requested review from carlescufi

Assignees

@nordic-krch nordic-krch

Labels
area: Logging bug The issue is a bug, or the PR is fixing a bug Hotfix Fix for issues blocking development, i.e. CI issues, tests failing in CI, etc. Regression Something, which was working, does not anymore Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

tests: logging: uart: log_core_additional/logging.async failing
7 participants
@ycsin @cfriedt @jhedberg @nordic-krch @nordicjm @nashif @zephyrbot