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

drivers: console: uart_console.c: suggestion to add new line for unix or linux host #51805

Merged

Conversation

sj-goh
Copy link
Contributor

@sj-goh sj-goh commented Oct 31, 2022

This is a suggestion to use console_getline() for uart, that is connected to pc with unix or linux OS.
Add case '\n', so that new line can be detected.

Signed-off-by shun.jing.goh@gmail.com

@sj-goh sj-goh changed the title drivers: console: uart_console.c: suggestion to add new line or unix or linux host drivers: console: uart_console.c: suggestion to add new line for unix or linux host Nov 7, 2022
@sj-goh
Copy link
Contributor Author

sj-goh commented Jan 3, 2023

Hi, any updates? How do i add active reviewers? Thank you.

Copy link
Member

@dcpleung dcpleung left a comment

Choose a reason for hiding this comment

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

Please follow the contribution guide on writing commit messages: https://docs.zephyrproject.org/latest/contribute/guidelines.html#commit-guidelines

Also, have you tried this on both Linux and Windows terminals? Windows' line breaks are \r\n which means this changes will match the same line twice.

@sj-goh sj-goh force-pushed the uart_console_unix_new_line branch 3 times, most recently from ba0acb2 to ccd4b4a Compare January 29, 2023 07:59
@sj-goh
Copy link
Contributor Author

sj-goh commented Jan 29, 2023

Hi @dcpleung,

thank you for the review.

I do not have a window terminal, but i am able to test \r\n by using a input file.
You are right, the previous commit prints two line.
I have made some adjustments, by checking the previous character.

However, the double line problem will still occur, if the terminal encodes enter key as \n\r.

Is there a general define for terminal endings?

@dcpleung
Copy link
Member

Since Windows is doing \r\n, as long as we can do both \r\n' and \n', this should be fine.

dcpleung
dcpleung previously approved these changes Jan 30, 2023
@dcpleung
Copy link
Member

dcpleung commented Feb 8, 2023

Please fix the CI errors.

@sj-goh
Copy link
Contributor Author

sj-goh commented Feb 11, 2023

Hi @dcpleung,

thank you for the feedback.

i don't know how to fix that error.

Even if i revert my changes, the error is still there.
Thus, i am sure, that this is not related to my PR.

I have pulled locally:
main d00f9b594b [origin/main] kernel/work: Fix race under with delayed work item cancellation
Here is the cmd that i have used:
west build -b qemu_x86_64 -T tests/libraries/mpsc_pbuf/lib.mpsc_pbuf_concurrent -p auto -t run

The test gives different result for each excution.
I would often get stuck at

START - test_stress_preemptions_high_consumer
3% remaining:0 ms ms

or sometimes kernel panic after the test is executed.

TESTSUITE mpsc_pbuf_concurrent succeeded

------ TESTSUITE SUMMARY START ------

SUITE FAIL -  95.24% [log_buffer]: pass = 20, fail = 1, skip = 0, total = 21 duration = 1.225 seconds
 - PASS - [log_buffer.test_benchmark_item_put] duration = 0.029 seconds
 - PASS - [log_buffer.test_benchmark_item_put_data] duration = 0.012 seconds
 - PASS - [log_buffer.test_benchmark_item_put_ext] duration = 0.008 seconds
 - PASS - [log_buffer.test_item_alloc_commit] duration = 0.036 seconds
 - PASS - [log_buffer.test_item_alloc_commit_saturate] duration = 0.002 seconds
 - PASS - [log_buffer.test_item_alloc_preemption] duration = 0.002 seconds
 - PASS - [log_buffer.test_item_max_alloc] duration = 0.001 seconds
 - PASS - [log_buffer.test_item_put_ext_no_overwrite] duration = 0.002 seconds
 - PASS - [log_buffer.test_item_put_ext_saturate] duration = 0.004 seconds
 - PASS - [log_buffer.test_item_put_no_overwrite] duration = 0.001 seconds
 - PASS - [log_buffer.test_item_put_overwrite] duration = 0.002 seconds
 - PASS - [log_buffer.test_item_put_saturate] duration = 0.003 seconds
 - PASS - [log_buffer.test_item_put_word_ext_overwrite] duration = 0.001 seconds
 - PASS - [log_buffer.test_overwrite] duration = 0.003 seconds
 - PASS - [log_buffer.test_overwrite_consistency] duration = 1.073 seconds
 - PASS - [log_buffer.test_overwrite_while_claimed] duration = 0.003 seconds
 - PASS - [log_buffer.test_overwrite_while_claimed2] duration = 0.003 seconds
 - FAIL - [log_buffer.test_pending_alloc] duration = 0.033 seconds
 - PASS - [log_buffer.test_put_data_overwrite] duration = 0.002 seconds
 - PASS - [log_buffer.test_put_while_claim] duration = 0.002 seconds
 - PASS - [log_buffer.test_utilization] duration = 0.003 seconds

SUITE SKIP -   0.00% [mpsc_pbuf_concurrent]: pass = 0, fail = 0, skip = 3, total = 3 duration = 0.003 seconds
 - SKIP - [mpsc_pbuf_concurrent.test_stress_preemptions_high_consumer] duration = 0.001 seconds
 - SKIP - [mpsc_pbuf_concurrent.test_stress_preemptions_low_consumer] duration = 0.001 seconds
 - SKIP - [mpsc_pbuf_concurrent.test_stress_preemptions_mid_consumer] duration = 0.001 seconds

------ TESTSUITE SUMMARY END ------

===================================================================
PROJECT EXECUTION FAILED
ASSERTION FAIL [z_spin_lock_valid(l)] @ WEST_TOPDIR/zephyr/include/zephyr/spinlock.h:148
	Recursive spinlock 0x120348
E: RAX: 0x0000000000000004 RBX: 0x0000000000120330 RCX: 0x0000000000000001 RDX: 0x0000000000000000
E: RSI: 0x0000000000000094 RDI: 0x000000000010fe32 RBP: 0x000000000011b3c0 RSP: 0x000000000011b358
E:  R8: 0x0000000000000001  R9: 0x0000000000000000 R10: 0x00000000ffffffff R11: 0x000000000011b1f0
E: R12: 0x00000000fffffff5 R13: 0x0000000000000000 R14: 0x0000000000000003 R15: 0x0000000000000206
E: RSP: 0x000000000011b358 RFLAGS: 0x0000000000000002 CS: 0x0018 CR3: 0x0000000000141000
E: RIP: 0x0000000000104c98
E: >>> ZEPHYR FATAL ERROR 4: Kernel panic on CPU 0
E: Current thread: 0x113770 (unknown)
E: Halting system

on rare occasion, the test is passed:

------ TESTSUITE SUMMARY END ------

===================================================================
PROJECT EXECUTION SUCCESSFUL

I am not sure if this is because of my low spec ubuntu vm.

Some fix is introduced in at #38777.
Should we create another issue to fix the test again?

I would suggest to remove the test suite, until it is stable.
There are also some other PR that fails due to this test.
#54676, #54639, #54594

Add case \n, so that new line from unix or linux host can be detected.

Signed-off-by: Shun Jing Goh <shun.jing.goh@gmail.com>
@carlescufi carlescufi merged commit 9d7c51e into zephyrproject-rtos:main Apr 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants