[riot-notifications] [RIOT-OS/RIOT] usbus: Add automated test (#12267)

Dylan Laduranty notifications at github.com
Wed Sep 18 20:27:06 CEST 2019


dylad commented on this pull request.

Some nitpicks
Tested work on SAME54-XPRO using tests/usbus
CDC-ECM is still working too.

> +{
+    switch (state) {
+        case TEST_REQ_PHASE_IDLE:
+            return "idle";
+        case TEST_REQ_PHASE_START_GET:
+            return "start get";
+        case TEST_REQ_PHASE_START_SET:
+            return "start set";
+        case TEST_REQ_PHASE_OUTDATA:
+            return "OUT data";
+        case TEST_REQ_PHASE_INDATA:
+            return "IN data";
+        case TEST_REQ_PHASE_OUTACK:
+            return "OUT Ack";
+        case TEST_REQ_PHASE_INACK:
+            return "IN Ack";

missing default statement

> +    /* Validate direction and content for the current state */
+    switch (req_phase) {
+        case TEST_REQ_PHASE_START_GET:
+            _validate_get_req_start(ep);
+            _handle_data(dev, ep, len);
+            break;
+        case TEST_REQ_PHASE_START_SET:
+            _validate_set_req_start(dev, ep, len);
+            break;
+        case TEST_REQ_PHASE_OUTACK:
+            _validate_out_ack(dev, ep);
+            break;
+        case TEST_REQ_PHASE_INACK:
+            _validate_in_ack(dev, ep, len);
+            break;
+        default:

please add a DEBUG statement

> @@ -0,0 +1,22 @@
+#!/usr/bin/env python3
+
+# Copyright (C) 2016 Kaspar Schleiser <kaspar at schleiser.de>
+# Copyright (C) 2016 Takuo Yonezawa <Yonezawa-T2 at mail.dnp.co.jp>

Hmmmmm, really ?

> +#define ENABLE_DEBUG    (0)
+#include "debug.h"
+
+static usbdev_mock_t usbdev_mock;
+static uint8_t _in_buf[256];    /* "host" in */
+static uint8_t _out_buf[64];    /* "host" out */
+
+static const usbdev_driver_t testdriver;
+
+static usbdev_mock_t *_ep2dev(usbdev_ep_t *ep)
+{
+    return (usbdev_mock_t *)ep->dev;
+}
+
+void usbdev_init_lowlevel(void)
+{}

```suggestion
{

}
```

> +                       usbdev_mock_ready_cb_t ready_cb)
+{
+    memset(&usbdev_mock, 0, sizeof(usbdev_mock));
+    usbdev_mock.usbdev.driver = &testdriver;
+
+    usbdev_mock.esr_cb = esr_cb;
+    usbdev_mock.ep_esr_cb = ep_esr_cb;
+    usbdev_mock.ready_cb = ready_cb;
+}
+
+static void _init(usbdev_t *usbdev)
+{
+    usbdev_mock_t *dev = (usbdev_mock_t *)usbdev;
+
+    /* Init fake usbdev */
+

Missing call or forgot to remove comment ?

> +typedef struct {
+    usbdev_ep_t ep;                 /**< Generic endpoint struct        */
+    usbdev_mock_ep_state_t state;   /**< Endpoint state                 */
+    size_t available;               /**< Bytes available in the buffer  */
+    uint8_t *buf_start;             /**< Start location of the buffer   */
+} usbdev_mock_ep_t;
+
+/**
+ * @brief usbdev mock device forward declaration
+ */
+typedef struct usbdev_mock usbdev_mock_t;
+
+/**
+ * @brief usbdev mock device callback for esr event checking
+ *
+ * @param dev   usbdev mock device state

You should change this description
Same for all functions within this file using dev arg

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/12267#pullrequestreview-290106527
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190918/335cde8d/attachment-0001.htm>


More information about the notifications mailing list