Zephyr QSpy port

[vChavezB]

I have made a preliminary port for qspy for zephyr and tested it with an NRF52832 board could you check it out if it works on your side? On my side it works, some packets are having errors but I suspect its because of it running at 115200 kbps, since I noticed that the transitions happen really fast (I suspect it could be a problem with the random timing generator (?) )

Attached as a text block is a patch the BSP of the zephyr DPP example, and in addition the Qspy sources must be enabled in the cmakelist.

diff --git a/examples/zephyr/dpp/src/bsp.cpp b/examples/zephyr/dpp/src/bsp.cpp
index 8b8a0b1..5f41518 100644
--- a/examples/zephyr/dpp/src/bsp.cpp
+++ b/examples/zephyr/dpp/src/bsp.cpp
@@ -33,6 +33,8 @@
 #include "bsp.hpp"

 #include <drivers/gpio.h>
+#include <drivers/uart.h>
+#include <sys/reboot.h>
 // add other drivers if necessary...

 // The devicetree node identifier for the "led0" alias.
@@ -135,7 +137,7 @@ void BSP::displayPhilStat(uint8_t n, char const *stat) {
     else {
         ledOff();
     }
-    printk("Philo[%d]->%s\n", n, stat);
+    //printk("Philo[%d]->%s\n", n, stat);

     QS_BEGIN_ID(PHILO_STAT, AO_Philo[n]->m_prio) // app-specific record begin
         QS_U8(1, n);  // Philosopher number
@@ -179,25 +181,63 @@ namespace QP {
 // QF callbacks ==============================================================
 void QF::onStartup(void) {
     k_timer_start(&QF_tick_timer, K_MSEC(1), K_MSEC(1));
-    printk("QF::onStartup\n");
+    //printk("QF::onStartup\n");
 }
 //............................................................................
 void QF::onCleanup(void) {
-    printk("QF::onCleanup\n");
+    //printk("QF::onCleanup\n");
 }

 // QS callbacks ==============================================================
 #ifdef Q_SPY
+static const struct device *uart_console_dev;
+
 //............................................................................
+constexpr size_t qspy_stack_size = 2048;
+K_THREAD_STACK_DEFINE(qspy_stack, qspy_stack_size); /* stack storage */
+k_thread qspy_thread_handler;
+static void qspy_thread(void *p1, void *p2, void *p3){
+    while(1){
+        uint16_t len;
+        uint8_t const *buf = QS::getBlock(&len); // get continguous block of data
+        while (buf != nullptr) { // data available?
+            for(auto i = 0;i!=len;i++)
+            {
+                uart_poll_out(uart_console_dev,buf[i]); 
+            }        
+            len = 0xFFFFU; // big number to get as many bytes as available
+            buf = QS::getBlock(&len); // try to get more data
+        }
+        unsigned char in_char;
+        const int res = uart_poll_in(uart_console_dev,&in_char);
+        if(res==0)
+        {
+            QS::rxPut(in_char);
+            QS::rxParse();
+        }
+    }
+}
+
 bool QS::onStartup(void const *arg) {
-    static uint8_t qsTxBuf[2*1024]; // buffer for QS transmit channel
+    static uint8_t qsTxBuf[4*2048]; // buffer for QS transmit channel
     static uint8_t qsRxBuf[100];    // buffer for QS receive channel

     initBuf  (qsTxBuf, sizeof(qsTxBuf));
     rxInitBuf(qsRxBuf, sizeof(qsRxBuf));
-
-    //TBD...
-
+    uart_console_dev = DEVICE_DT_GET(DT_CHOSEN(zephyr_console));
+    //TODO assert if nullptr
+    k_thread_create(&qspy_thread_handler,
+                    qspy_stack,
+                    qspy_stack_size,
+                    &qspy_thread,
+                    nullptr, // p1
+                    nullptr,    // p2
+                    nullptr,    // p3
+                    14,      // Zephyr priority */
+                    K_ESSENTIAL,        // thread options */
+                    K_NO_WAIT); // start immediately */
+
+    //TODO assert if could not create thread
     return true; // return success
 }
 //............................................................................
@@ -205,17 +245,25 @@ void QS::onCleanup(void) {
 }
 //............................................................................
 QSTimeCtr QS::onGetTime(void) {  // NOTE: invoked with interrupts DISABLED
-    //TBD...
-    return 0U;
+    return k_uptime_get_32();
 }
 //............................................................................
 void QS::onFlush(void) {
-    //TBD...
+    uint16_t len = 0xFFFFU; // big number to get as many bytes as available
+    uint8_t const *buf = QS::getBlock(&len); // get continguous block of data
+    while (buf != nullptr) { // data available?
+        for(auto i = 0;i!=len;i++)
+        {
+            uart_poll_out(uart_console_dev,buf[i]); 
+        }        
+        len = 0xFFFFU; // big number to get as many bytes as available
+        buf = QS::getBlock(&len); // try to get more data
+    }
 }
 //............................................................................
 //! callback function to reset the target (to be implemented in the BSP)
 void QS::onReset(void) {
-    //???sys_reboot();
+    sys_reboot(SYS_REBOOT_COLD);
 }
 //............................................................................
 //! callback function to execute a user command (to be implemented in BSP)

[vChavezB]

I think it now works correctly. Please see the example I built here

https://github.com/vChavezB/dpp_qspy_zephyr

Feel free to test it and see if it works for any other boards. As it uses the Zephyr API to setup QSPy it should work out of the box for any Zephyr compatible board.

[quantum-leaps]

Hi there, Thanks for the updates. I've merged and pushed your changes with some modifications. Please check it out.

I've tested the QSPY on nucleo_h743zi. Output (QS-TX) seems to be working. But input (QS-RX) does not. This would be good to have for unit testing with QUTest.

Also, the time stamp is of so imprecise that it's of very limited use. It would be much better to have microsecond-level granularity.

Perhaps you could help with these issues? Obviously getting QS-RX to work is the higher prioirty.

Finally, I don't understand Zephyr config and how to build different configurations (like the Spy).

--MMS

[vChavezB]

Well what I tested with QS-RX was only sending the reboot command from the QSpy host application. How could I test more extensively QS-RX?

It would be much better to have microsecond-level granularity.

I am not 100% familiar with the Zephyr API, I will have a look if there is something better than k_uptime_get_32.

how to build different configurations (like the Spy).

What I did in https://github.com/vChavezB/dpp_qspy_zephyr is made qpcpp as an external module. Basically you define a zephyr directory and a yaml file that defines where the CMakelists to build the module is located and the Kconfig file. I made a KConfig file to enable the QPCPP module and enable QSPY. The options from KConfig generate definitions which then can be used by cmake.

[vChavezB]

I replace the polling api for the interrupt api, could you try the following:

Add in prj.conf the following

CONFIG_SERIAL=y
CONFIG_UART_INTERRUPT_DRIVEN=y

And apply the next patch to the qpcpp repo

diff --git a/examples/zephyr/dpp/src/bsp.cpp b/examples/zephyr/dpp/src/bsp.cpp
index f7a907f5..6b12c266 100644
--- a/examples/zephyr/dpp/src/bsp.cpp
+++ b/examples/zephyr/dpp/src/bsp.cpp
@@ -190,6 +190,19 @@ void QF::onCleanup(void) {
 #include <drivers/uart.h>

 static const struct device *uart_console_dev;
+static std::uint8_t uart_buf[1024];
+static void uart_cb(const struct device *x, void *user_data)
+{
+   uart_irq_update(x);
+   int data_length = 0;
+   if (uart_irq_rx_ready(x)) {
+       data_length = uart_fifo_read(x, uart_buf, sizeof(uart_buf));
+        for(auto i=0;i<data_length;i++)
+        {
+            QS::rxPut(uart_buf[i]);
+        }
+   }
+}

 //............................................................................
 static K_THREAD_STACK_DEFINE(qspy_stack, 1024); // stack storage
@@ -208,10 +221,12 @@ static void qspy_thread(void *p1, void *p2, void *p3){
         }

         // receive bytes...
+        /*
         std::uint8_t b;
         while (uart_poll_in(uart_console_dev, &b) == 0) {
             QS::rxPut(b);
         }
+        */
         QS::rxParse();
     }
 }
@@ -234,6 +249,9 @@ bool QS::onStartup(void const *arg) {
                     QF_MAX_ACTIVE, // lowest priority
                     K_ESSENTIAL,   // thread options
                     K_NO_WAIT);    // start immediately
+
+    uart_irq_callback_user_data_set(uart_console_dev, uart_cb, nullptr);
+   uart_irq_rx_enable(uart_console_dev);
     //TODO assert if could not create thread
     return true; // return success
 }

[vChavezB]

Another thing I noticed is that ticks per second should probably be 1000 instead of 100 in

https://github.com/QuantumLeaps/qpcpp/blob/d9f277b0ecb8d0dc05cd3c702746010dd37c4534/examples/zephyr/dpp/src/bsp.hpp#L41

Since the timer is set to 1 msec here https://github.com/QuantumLeaps/qpcpp/blob/d9f277b0ecb8d0dc05cd3c702746010dd37c4534/examples/zephyr/dpp/src/bsp.cpp#L181

[vChavezB]

For the clock measurement the API k_cycle_get_32 can get the current ticks of the clock and converted to nanoseconds with https://docs.zephyrproject.org/1.14.1/reference/kernel/timing/clocks.html#c.SYS_CLOCK_HW_CYCLES_TO_NS

[quantum-leaps]

Hi vChavezB, Thanks a lot for the effort.

At this point, your QS implementation is a hybrid of polled transmission and interrupt-driven reception. This means that the expensive qspy_thread is still needed (with its stack as big as the QS buffers!). Would it be possible (and perhaps more efficient) to do everything in a consistent, interrupt-driven manner? (Although I don't know where to call QS::rxPartse()...) It's really too bad that Zephyr does not provide any hooks into the idle thread, which is paid for already.

Second, thanks a lot for creating the qpcpp_zephyr repo. If we really want to do this right, it seems that qpcpp (and also qpc) should become zephyr modules. Could you perhaps help to make that happen?

Alternatively, if qpcppp will not be accepted (or literal copying of qpcpp source code to Zephyr makes no sense), a repo like qpcpp_zephyr logically makes more sense from the same source as qpcpp itself, that is from QuantumLeaps. Please let me know what you think.

--MMS

[vChavezB]

What I wonder is why the QS-RX does not work correctly in polled mode, perhaps the buffer gets overwritten due to the higher priority of the other threads? Could you tell me how I can test more extensively QS-RX? The only thing I have done until know is send the Reset command to test Qs-RX but perhaps I can test more extensively the polling method and see what other APIs are there in Zephyr to set up the uart driver correctly.

In addition there seems to be an idle hook but only when power management is activated, I will look what else can be done so this hybrid implementation can be optimized About the Microseconds here is a snippet that should give you microseconds in zephyr

const std::uint32_t hw_cycle_count = k_cycle_get_32();
const std::uint32_t microseconds = k_cyc_to_ns_floor64(hw_cycle_count)/1000U;

I think it should not be a problem to make the qpcpp repo compatible as a zephyr module. I just need to copy the zephyr dir to the root of the repo and set where the manifest and config file for west should be located. I will check it this week and will let you know how it goes.

Integrating qppcpp to zephyr would need to pass their process

https://docs.zephyrproject.org/latest/contribute/external.html#external-src-process

And lastly, I have no problems in contributing to the zephyr port as my goal is to use it for BLE applications with the Nordic Connect SDK :)

Regards Victor

[quantum-leaps]

Could you tell me how I can test more extensively QS-RX? The only thing I have done until know is send the Reset command to test Qs-RX but perhaps I can test more extensively

Directly from QSPY you can test: target reset ('r' keystroke) and target info ('i' keystroke). You can test QS-RX more extensively with QView. You can just open up QView and interact with the target via the provided user interface. For example, you can change the filters, send events to the target, etc.

I think it should not be a problem to make the qpcpp repo compatible as a zephyr module.

That would be great. For completeness, similar Zephyr module should be made for qpc as well...

Thanks a lot for your help, --MMS

[vChavezB]

As you have already modified the repo with the port I will close this issue. I will test the changes and I if I notice something wrong I can let you know.