apps/testcase/systemio/itc: add new UART TCs #5050

deepaksrma · 2021-07-12T09:10:21Z

add new TCs to test UART rxavailable, txready and txempty APIs.

Signed-off-by: Deepak sharma [email protected]

tizen-build · 2021-07-12T09:12:39Z

Target : [76177faf3303154c2f3bd04e9a2b7f2782da9088] - Code Rule Check OK.

apps/examples/testcase/ta_tc/systemio/itc/itc_uart.c

lib/libc/termios/lib_tccheckfifo.c

jeongchanKim

Please split the commit as 2.
1st is for changing the framework and lib, 2nd is for tc.

lib/libc/termios/lib_tccheckfifo.c

apps/examples/testcase/ta_tc/systemio/itc/itc_uart.c

lib/libc/termios/lib_tccheckfifo.c

framework/src/iotbus/iotbus_uart.c

lib/libc/termios/lib_tccheckfifo.c

framework/src/iotbus/iotbus_uart.c

apps/examples/testcase/ta_tc/systemio/itc/itc_uart.c

tizen-build · 2021-07-12T14:06:36Z

Target : [73fbdc8c349651c513a4d7a1ea782cb9f73ab3cf] - Code Rule Check OK.

framework/include/iotbus/iotbus_error.h

apps/examples/testcase/ta_tc/systemio/itc/itc_uart.c

tizen-build · 2021-07-12T18:54:37Z

Target : [cb8df92dc393026b806f82950f50cdc69db9d0d4] - Code Rule Check OK.

tizen-build · 2021-07-12T19:00:36Z

Target : [d12a0c61879e5ad09bd12d7b54b8a109d8474f56] - Code Rule Check OK.

framework/src/iotbus/iotbus_uart.c

lib/libc/termios/lib_tccheckfifo.c

sunghan-chang · 2021-08-05T14:35:05Z

@deepaksrma Please let me know the plan of this PR.

deepaksrma · 2021-08-06T14:10:42Z

@deepaksrma Please let me know the plan of this PR.

will update this after the merging of following PR :
#5101

lib/libc/termios/lib_tccheckfifo.c

os/include/termios.h

- add new APIs in iotbus to test UART TCs, APIs are: iotbus_uart_rxavailable() iotbus_uart_txready() iotbus_uart_txempty() - add new TCs to test UART rxavailable, txready and txempty APIs. Signed-off-by: Deepak Sharma <[email protected]>

- add new TCs to test UART rxavailable, txready and txempty APIs. Signed-off-by: Deepak Sharma <[email protected]>

tizen-build · 2021-09-07T13:39:34Z

Target : [23a21a7] - Code Rule Check OK.

sunghan-chang · 2021-09-08T03:02:14Z

os/arch/arm/src/amebad/amebad_serial.c

+static bool g_tc_rxavailable;
+static bool g_tc_rxavailable_res;	// used to hold the result of rxavailable TC


I don't agree adding codes here for tc.

please check my comment below and let me know what you think?
#5050 (comment)

sunghan-chang · 2021-09-08T03:03:01Z

apps/examples/testcase/ta_tc/systemio/itc/itc_uart.c

+/*	itc_systemio_iotbus_uart_txready();
+	itc_systemio_iotbus_uart_txempty();


Why do you comment them out?

mistakenly added, will uncomment

sunghan-chang · 2021-09-08T03:05:00Z

os/arch/arm/src/amebad/amebad_serial.c

+	case TIOCS_AVAIL:
+		g_tc_rxavailable = 1;
+		g_tc_rxavailable_res = 0;
+		rtl8721d_up_send(dev, 'a');
+		sleep(1);
+		ret = g_tc_rxavailable_res;
+		break;
+
+	case TIOCS_READY:
+		ret = rtl8721d_up_txready(dev);
+		break;
+
+	case TIOCS_EMPTY:
+		ret = rtl8721d_up_txempty(dev);
+		break;


How about other boards like imxrt? Does it return invalid or not supported or already impleneted?

Are they only used in the tc for coverage? Then let's cover them with TC config.

will add this in other boards after finalizing for this board

sunghan-chang · 2021-09-08T03:06:22Z

os/arch/arm/src/amebad/amebad_serial.c

+		g_tc_rxavailable = 1;
+		g_tc_rxavailable_res = 0;
+		rtl8721d_up_send(dev, 'a');
+		sleep(1);
+		ret = g_tc_rxavailable_res;


I don't know what you want to do. Could you leave comment to explain?

for rxavailable TC, my approach is:

rxavailable returns true when there is data in rx fifo and when data comes on the tx line in loopback mode then it enables the RxIrq and it goes to uart_recvchars(), where it checks whether data is available in rx fifo or not.

so to check the data in rx fifo, we need to send the data first, which we have sent through rtl8721d_up_send() and before sending the data we need to enable the g_tc_rxavailable so that g_tc_rxavailable_res can collect the result for our sent data and when it collects the result then immediately turn off the g_tc_rxavailable, which means it won't collect the result further.

please let me know what you think about this approach.
I'll add the comments once we agree on the approach.

@kishore-sn @sangam-swami @jeongchanKim please review this PR.

I understand they are "begin" and "end" of tc to get valid data. Am I right?

It's the loopback test. There is no way to get another data. Why do you need begin and end?

Even it's loopback mode, sender and receiver should be implemented using existed APIs without more variables. If it's impossible, it means our APIs are not enough to use serial functionality. Then we should add them as public API, not tc.

How about changing this implementation as follows:
Remove all global variables.

case TIOCS_AVAIL:
rtl8721d_up_send(dev, 'a');
sleep(1);
ret = rtl8721d_up_rxavailable(dev);
break;

On the other hand, I think we can change it further to make the code much cleaner.

In the driver we do:
case TIOCS_AVAIL:
ret = rtl8721d_up_rxavailable(dev);
break;

In the test case file we do:
iotbus_uart_context_h h_uart = iotbus_uart_init(DEVPATH);
ret = iotbus_uart_control_loopback(h_uart, LOOPBACK_ON);
ret = iotbus_uart_write(h_uart, sz_input_text, sizeof(sz_input_text));
ret = iotbus_uart_rxavailable(h_uart);

we can not do like this:

case TIOCS_AVAIL: rtl8721d_up_send(dev, 'a'); sleep(1); ret = rtl8721d_up_rxavailable(dev); break;

because when we sent the data in loopback mode then it comes on the RX FIFO immediately and generates the interrupt,
and calls uart_recvchars() where it checks whether data is available in RX FIFO or not, if data is available then it removes the data from RX FIFO and copies it to buffer so the following lines won't work, because when it comes to rtl8721d_up_rxavailable(), then data is not in RX FIFO because it has been transferred to buffer.

ret = rtl8721d_up_rxavailable(dev); break;

ok.
Then we need to keep a global variable. But its better to keep a single global variable and protect it with a semaphore. Add a new ioctl to reset global variable.

bool g_rxavail' 'sem_t g_rxavail_sem

TCIOC_RXAVAIL_RESET:
take g_rxavail_sem
g_rxavail = false
give g_rxavail_sem

TCIOC_RXAVAIL:
take g_rxavail_sem
ret = g_rxavail;
give g_rxavail_sem

And in rtl8721d_up_rxavailable()

take g_rxavail_sem
g_rxavail = ret;
give g_rxavail_sem

amogh-samsung · 2021-09-08T09:17:21Z

apps/examples/testcase/ta_tc/systemio/itc/itc_uart.c

@@ -865,6 +942,10 @@ int itc_uart_main(void)
 	itc_systemio_iotbus_uart_set_mode_write_read_p();


Could you please add a "TODO" comment for line 939
//itc_systemio_iotbus_uart_write_read_flush_read_p();// TASH being hanged on execution of the testcase
If this particular testcase is not resolved and is left to be resolved later.

kishore-sn · 2021-09-16T05:04:41Z

os/arch/arm/src/amebad/amebad_serial.c

@@ -175,6 +175,8 @@
 */

 static serial_t* sdrv[MAX_UART_INDEX + 1] = {NULL, NULL, NULL, NULL, NULL}; //uart 0~3, uart2 is configured as log uart
+static bool g_tc_rxavailable;


Please add a comment explaining the meaning of this variable.

kishore-sn · 2021-09-16T05:15:16Z

os/arch/arm/src/amebad/amebad_serial.c

+		g_tc_rxavailable = 1;
+		g_tc_rxavailable_res = 0;
+		rtl8721d_up_send(dev, 'a');
+		sleep(1);
+		ret = g_tc_rxavailable_res;


How about changing this implementation as follows:
Remove all global variables.

case TIOCS_AVAIL:
rtl8721d_up_send(dev, 'a');
sleep(1);
ret = rtl8721d_up_rxavailable(dev);
break;

kishore-sn · 2021-09-16T05:28:59Z

os/arch/arm/src/amebad/amebad_serial.c

+		g_tc_rxavailable = 1;
+		g_tc_rxavailable_res = 0;
+		rtl8721d_up_send(dev, 'a');
+		sleep(1);
+		ret = g_tc_rxavailable_res;


On the other hand, I think we can change it further to make the code much cleaner.

In the driver we do:
case TIOCS_AVAIL:
ret = rtl8721d_up_rxavailable(dev);
break;

In the test case file we do:
iotbus_uart_context_h h_uart = iotbus_uart_init(DEVPATH);
ret = iotbus_uart_control_loopback(h_uart, LOOPBACK_ON);
ret = iotbus_uart_write(h_uart, sz_input_text, sizeof(sz_input_text));
ret = iotbus_uart_rxavailable(h_uart);

sunghan-chang · 2022-07-20T11:11:20Z

@sangam-swami Could you manage this PR?

tizen-build · 2023-03-21T02:16:29Z

Target : [23a21a7] - Code Rule Check OK.

abhishek-samsung reviewed Jul 12, 2021

View reviewed changes

apps/examples/testcase/ta_tc/systemio/itc/itc_uart.c Outdated Show resolved Hide resolved

apps/examples/testcase/ta_tc/systemio/itc/itc_uart.c Outdated Show resolved Hide resolved

lib/libc/termios/lib_tccheckfifo.c Outdated Show resolved Hide resolved

jeongchanKim reviewed Jul 12, 2021

View reviewed changes

dineshdas-samsung reviewed Jul 12, 2021

View reviewed changes

lib/libc/termios/lib_tccheckfifo.c Outdated Show resolved Hide resolved

sangam-swami reviewed Jul 12, 2021

View reviewed changes

apps/examples/testcase/ta_tc/systemio/itc/itc_uart.c Outdated Show resolved Hide resolved