[riot-notifications] [RIOT-OS/RIOT] sx127x: add several NETOPT for GNRC LoRaWAN (#11736)

Alexandre Abadie notifications at github.com
Fri Jun 28 08:15:05 CEST 2019


aabadie requested changes on this pull request.

I tested this PR and it works. I still have a usability issue the syncword command, see below.

> +    if (strstr(argv[1], "get") != NULL) {
+        netdev->driver->get(netdev, NETOPT_SYNCWORD, &syncword,
+                            sizeof(syncword));
+        printf("Syncword: %02x\n", (int)syncword);
+        return 0;
+    }
+
+    if (strstr(argv[1], "set") != NULL) {
+        if (argc < 3) {
+            puts("usage: syncword set <syncword>");
+            return -1;
+        }
+        syncword = atoi(argv[2]);
+        netdev->driver->set(netdev, NETOPT_SYNCWORD, &syncword,
+                            sizeof(syncword));
+        printf("New syncword set\n");

can be replaced by puts, or eventually print the value that has been set.

> +    if (argc < 2) {
+        puts("usage: channel <get|set>");
+        return -1;
+    }
+
+    netdev_t *netdev = (netdev_t *)&sx127x;
+    uint16_t rx_timeout;
+    if (strstr(argv[1], "set") != NULL) {
+        if (argc < 3) {
+            puts("usage: rx_timeout set <rx_timeout>");
+            return -1;
+        }
+        rx_timeout = atoi(argv[2]);
+        netdev->driver->set(netdev, NETOPT_RX_SYMBOL_TIMEOUT, &rx_timeout,
+                            sizeof(rx_timeout));
+        printf("New rx_timeout set\n");

can be replaced by puts

> +
+    netdev_t *netdev = (netdev_t *)&sx127x;
+    uint8_t syncword;
+    if (strstr(argv[1], "get") != NULL) {
+        netdev->driver->get(netdev, NETOPT_SYNCWORD, &syncword,
+                            sizeof(syncword));
+        printf("Syncword: %02x\n", (int)syncword);
+        return 0;
+    }
+
+    if (strstr(argv[1], "set") != NULL) {
+        if (argc < 3) {
+            puts("usage: syncword set <syncword>");
+            return -1;
+        }
+        syncword = atoi(argv[2]);

It's very counter intuitive regarding the `get` subcommand: here the input value is interpreted as an int but the `get` subcommand returns it as an hex.
So it's either everything is entered/printed as int or everything is entered/printed as hex.

-- 
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/11736#pullrequestreview-255598152
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190627/f78d7fd0/attachment.html>


More information about the notifications mailing list