USBUS: Initial work towards an USB stack #10916

bergzand · 2019-01-31T22:07:17Z

Contribution description

USBUS (Universal Serial Bus Unified Stack) is a generic USB stack specific for RIOT-os. It is a newly written USB stack making use of the usbdev module proposed in #9830. The idea is to have a single message/event based thread handling all the usb work.

Modularity is achieved by allowing different endpoint modules to register themselves with their specific settings. On startup, each module requiring USB endpoints have to request endpoints from USBUS and add a callback for handling the messages. Examples of modules that could be created are: USB CDC ACM (serial over USB), USB HID (sensors over USB) and USB CDC EEM (ethernet over USB).

Testing procedure

Run the examples/usbus_minimal on a samr21-xpro board. Plugging in the second (TARGET USB) in an USB host device (your desktop) should cause it to show up in the usb device listing with something like:

% lsusb  -d 046d:5678 -v

Bus 003 Device 018: ID 046d:5678 Logitech, Inc. 
Couldn't open device, some information will be missing
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               1.10
  bDeviceClass            0 
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0        64
  idVendor           0x046d Logitech, Inc.
  idProduct          0x5678 
  bcdDevice            0.00
  iManufacturer           3 
  iProduct                2 
  iSerial                 0 
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength            9
    bNumInterfaces          0
    bConfigurationValue     1
    iConfiguration          1 
    bmAttributes         0x80
      (Bus Powered)
    MaxPower              100mA

(I'm fully aware that I'm stealing one of the Logitech vendor codes, It's on my todo for #9830 to change it to something else).

Issues/PRs references

depends on #10915 and #9830

Todo:

auto_init (usbus: Initial simple auto init structure #11075)

dylad · 2019-02-01T12:51:53Z

@bergzand I'll have a look this week-end or next week.
I don't see any examples/usbus_minimal with this PR. Is this missing or will it come in a followup PR ?

bergzand · 2019-02-01T13:53:30Z

I don't see any examples/usbus_minimal with this PR. Is this missing or will it come in a followup PR ?

Must have forgotten to commit/push this, thanks for notifying me

dylad · 2019-02-01T20:40:06Z

sys/usb/usbus/usbus.c

+                usbus->addr = 0;
+                usbus->setup_state = USBUS_SETUPRQ_READY;
+                usbus->dev->driver->set(usbus->dev, USBOPT_ADDRESS, &usbus->addr, sizeof(uint8_t));
+                puts("Reset");


Huh, maybe something like usbus: event reset
Also why do you use puts() and not the DEBUG() function ?

Because of lazy probably, sorry :(

keestux · 2019-02-01T21:11:21Z

cpu/sam0_common/usb/usb.c

+                usbdev_ep_ready(ep, 0);
+            }
+            res = sizeof(usbopt_enable_t);
+            res = sizeof(usbopt_enable_t);


Duplicate line?

Yes, fixing this in #10915

keestux · 2019-02-01T21:16:09Z

cpu/sam0_common/include/sam_usb.h

+ */
+
+/**
+ * @defgroup    sys_uuid RFC 4122 compliant UUID's


Copy-paste error?

keestux · 2019-02-01T21:25:57Z

cpu/sam0_common/usb/usb.c

+        unsigned ep_num = bitarithm_lsb(USB->DEVICE.EPINTSMRY.reg);
+        UsbDeviceEndpoint *ep_reg = &USB->DEVICE.DeviceEndpoint[ep_num];
+        if (_ep_in_flags_set(ep_reg)) {
+            usbdev_ep_t *ep = &endpoints[_get_ep_num(ep_num, USB_EP_DIR_IN)];


Why not use _get_ep() ?

kaspar030 · 2019-02-01T22:02:53Z

sys/usb/usbus/usbus.c

+                      .content = { .ptr = usbdev } };
+
+        if (msg_send(&msg, usbus->pid) <= 0) {
+            puts("usbus: possibly lost interrupt.");


I'd seriously consider using thread flags for this. You'd disable this failure case entirely, thread flags cannot get lost.

An arriving message also sets a thread flag, so a thread can wait for both:

thread_flag_t flags = thread_flag_wait_any(0xffff); if (flags & THREAD_FLAG_MSG_WAITING) { msg_t msg while (msg_try_receive(&msg) == 1) { // handle netapi messages } } if (flags & YOUR_FLAG_SET_BY_ISR) { /// handle } }

I understand if you'd rather not go into this for now, can be changed later.

This is actually one of the issues where I'd really would like to have some feedback on. I've used messages here because it's what I'm most familiar with from gnrc, but using thread flags or events are both open to suggestions for me.

After some out-of-band discussion it became clear that it should be easy to move to thread flags with a flag for the generic usbdev ISR and a flag for endpoint specific ISR's. A uint32_t can keep track of the endpoints that require servicing (USB has 32 endpoints max, 16 IN and 16 OUT). This has as a consequence that usbus needs fast access to the usbdev endpoints, preferably O(1) complexity to resolve the thread flag back to an endpoint.

One solution here is to allocate the usbus_endpoint_t (not to be confused with usbdev_ep_t) as a static array. This could then reuse the USBDEV_NUM_ENDPOINTS, matching the number of endpoints allocated by the peripheral.

@kaspar030 I rewrote the loop to use thread flags. 2 flags are used as described above. Additionally I've added handling for events to allow signalling events to usb functionality from outside or inside the usbus thread.

keestux

Here are a few more comments. I hope to spend more time in the next days.

keestux · 2019-02-01T21:35:24Z

cpu/sam0_common/usb/usb.c

+
+static bool usb_enable_syncing(void)
+{
+    if (USB->DEVICE.SYNCBUSY.reg & USB_SYNCBUSY_ENABLE) {


Personally I like the .bit notation easier to read.

if (USB->DEVICE.SYNCBUSY.bit.ENABLE) {

Agreed, fixed in #10915

keestux · 2019-02-01T21:36:44Z

cpu/sam0_common/usb/usb.c

+
+static bool usb_swrst_syncing(void)
+{
+    if (USB->DEVICE.SYNCBUSY.reg & USB_SYNCBUSY_SWRST) {


if (USB->DEVICE.SYNCBUSY.bit.SWRST) {

Agreed, fixed in #10915

keestux · 2019-02-01T21:42:57Z

cpu/sam0_common/usb/usb.c

+                                   GCLK_CLKCTRL_GEN_GCLK0 |
+                                   (GCLK_CLKCTRL_ID(USB_GCLK_ID)));
+#elif defined(CPU_FAM_SAML21)
+    MCLK->AHBMASK.reg |= (MCLK_AHBMASK_USB);


Why the parenthesis?

Legacy probably, removed

keestux · 2019-02-01T22:01:29Z

cpu/sam0_common/usb/usb.c

+static inline void poweroff(void)
+{
+#if defined(CPU_FAM_SAMD21)
+    PM->AHBMASK.reg |= PM_AHBMASK_USB;


This should be clearing the bit. And also the APBBMASK is missing here. (Besides, the poweroff function is not used)

Ugh, let me check this one later, completely messed the function up

keestux · 2019-02-01T22:27:31Z

cpu/sam0_common/usb/usb.c

+
+void usb_attach(void)
+{
+    /* Datasheet is not clear whether device starts detached */


The comment does not belong here. This function is simply doing what it's been told to do.
BTW. I don't have a direct answer, but ASF usb init and Arduino bootloader do an attach. So I guess it is detached when the device starts.

This served as a reminder for myself to check this. As you said, this doesn't belong here, I'm removing it.

bergzand · 2019-02-02T16:02:01Z

rebased to the current #10915

dylad · 2019-02-02T20:19:17Z

@bergzand examples/usbus_minimal doesn't build.

/home/dylad/software/RIOT/examples/usbus_minimal/bin/arduino-zero/usbus.a(usbus.o): In function `usbus_init':
/home/dylad/software/RIOT/sys/usb/usbus/usbus.c:76: undefined reference to `driver'
collect2: error: ld returned 1 exit status

dylad · 2019-02-02T20:49:18Z

@bergzand nvm, I miss the

ifneq (,$(filter usbdev,$(USEMODULE)))
  USEMODULE += sam_usb
endif

I was able to successfully test it on arduino-zero.

keestux · 2019-02-02T21:54:07Z

Not much luck with my SODAQ board, which are almost the same as Arduino Zero.
I'v added the USEMODULE += sam_usb in the boards Makefile and enabled debug in usb.c and usbus.c

main(): This is RIOT! (Version: 2018.10-devel-527-gaf3ff6-rapper.fritz.box-usb-sodaq-one)
RIOT USB stack example application
usbus: starting thread 3
usbus: Adding string descriptor number 1 for: "USB config"
usbus: Adding string descriptor number 2 for: "USB device"
usbus: Adding string descriptor number 3 for: "RIOT-os.org"
Reset
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
Unhandled out 0: 0
Reset
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
Setting addres 35
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep0: possibly lost interrupt.
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep0: possibly lost interrupt.
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep0: possibly lost interrupt.
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep0: possibly lost interrupt.
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep0: possibly lost interrupt.
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep0: possibly lost interrupt.
usbus_ep: pid: 3
usbus_ep: pid: 3
usbus_ep: pid: 3
Unhandled in 0: 8
Unhandled in 0: 8
Unhandled in 0: 8
Unhandled in 0: 8
Unhandled in 0: 8
Unhandled in 0: 8

keestux · 2019-02-02T22:06:43Z

On the other hand, maybe I'm expecting too much at this point. What I do get is:

# lsusb -d 046d:5678 -v

Bus 001 Device 041: ID 046d:5678 Logitech, Inc. 
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               1.10
  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0        64
  idVendor           0x046d Logitech, Inc.
  idProduct          0x5678 
  bcdDevice            0.00
  iManufacturer           3 (error)
  iProduct                2 (error)
  iSerial                 0 
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength            9
    bNumInterfaces          0
    bConfigurationValue     1
    iConfiguration          1 (error)
    bmAttributes         0x80
      (Bus Powered)
    MaxPower              100mA
Device Status:     0x9cd0
  (Bus Powered)
  Debug Mode

bergzand · 2019-02-02T22:15:00Z

@keestux Did you try with debugging disabled? My experience with running with debug logging is that the delays caused by the serial transfer are enough to cause timing issues with USB.

It is still on my todo list to figure out how much debugging can be enabled without causing issues

keestux · 2019-02-03T15:04:20Z

@bergzand I think you're right. Without debug lsusb shows

# lsusb -d 046d:5678 -v

Bus 001 Device 043: ID 046d:5678 Logitech, Inc. 
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               1.10
  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0        64
  idVendor           0x046d Logitech, Inc.
  idProduct          0x5678 
  bcdDevice            0.00
  iManufacturer           3 RIOT-os.org
  iProduct                2 USB device
  iSerial                 0 
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength            9
    bNumInterfaces          0
    bConfigurationValue     1
    iConfiguration          1 USB config
    bmAttributes         0x80
      (Bus Powered)
    MaxPower              100mA
Device Status:     0x0000
  (Bus Powered)

On the RIOT device console it shows just two "Reset" messages:

main(): This is RIOT! (Version: 2018.10-devel-527-gaf3ff6-rapper.fritz.box-usb-sodaq-one)
RIOT USB stack example application
Reset
Reset

keestux · 2019-02-03T17:29:00Z

sys/include/usb/usbus.h

+#include <stdlib.h>
+#include "usb/usbdev.h"
+#include "usb.h"
+#include "usb/message.h"


This "usb/message.h" could just be "message.h" because it is included from a file in the same directory

I prefer the long form to prevent confusion for a future reader

keestux · 2019-02-03T17:36:55Z

sys/include/usb.h

+ * @brief USB peripheral device vendor ID
+ */
+#ifndef USB_CONFIG_VID
+#define USB_CONFIG_VID          (0x046d)


I know it is just temporary using a Logitech identifier. But why not use Arduino's vendor and product id until we have our own (if that's even feasible)? I think Arduino Zero has 0x2341:0x804d

I proposed to ask for a free PID from here in #9830

keestux · 2019-02-03T17:50:26Z

cpu/sam0_common/usb/usb.c

+    UsbDeviceEndpoint *ep_reg = &USB->DEVICE.DeviceEndpoint[ep->num];
+
+    if (ep->dir == USB_EP_DIR_OUT) {
+        ep_reg->EPINTENCLR.reg = USB_DEVICE_EPINTENCLR_TRCPT0 |


Sorry to nitpick, but I like symmetry. The code for enable/disable IRQ is very, very similar. Don't just reorder lines, Use the same formatting (the bitwise or at the end of line or at the next line).
I like to be able to quickly visually compare the two functions.

Ack, should be fixed

keestux · 2019-02-03T19:19:19Z

cpu/sam0_common/usb/usb.c

+        res->num = 0;
+    }
+    else {
+        for (unsigned idx = 1; idx < 8; idx++) {


Don't use magic constants. This 8, is that USBDEV_NUM_ENDPOINTS?

keestux · 2019-02-03T19:24:38Z

cpu/sam0_common/usb/usb.c

+void usbdev_init(usbdev_t *dev)
+{
+    /* Only one usb device on this board */
+    _usbdev = (sam0_common_usb_t *)dev;


Maybe add an assert(!_usbdev) so that nobody can get away with calling the function twice.

Reworked this to not have a hard dependency on the sam0_common_usb_t struct

keestux · 2019-02-03T21:34:02Z

sys/usb/usbus/usbus.c

+    else {
+        usb_descriptor_string_t desc;
+        desc.type = USB_TYPE_DESCRIPTOR_STRING;
+        mutex_lock(&usbus->lock);


It would help the reader to know what the lock is protection...
Otherwise one might ask why there is no lock in the section above.

These locks originate from the idea to have multiple threads interacting with the stack. I don't think that this was a very good idea, so for now it is one single thread and it should be possible to remove most if not all of these locks.

keestux · 2019-02-03T21:37:31Z

sys/usb/usbus/usbus.c

+            usbus_ep0_ready(usbus);
+        }
+        else {
+            usbus_ep0_ready(usbus);


Nothing has been "put". Is this OK then? To do usbus_ep0_ready?

It is. Some times you just want to notify things without send any data.

keestux · 2019-02-03T21:44:29Z

sys/usb/usbus/usbus.c

+    return pkt->length > usbus->in->len ? usbus->in->len : pkt->length;
+}
+
+void recv_setup(usbus_t *usbus, usbdev_ep_t *ep)


You probably know better than I, do we have a function naming convention? I mean, since C has a single name space, there could easily arise a name conflict with functions named like this.
If this becomes a static function then there is no problem, of course.

Should probably also be static

do we have a function naming convention?

Yes, public (non-static) functions should be called module_function() (e.g., usbus_recv_setup()). static functions should start with underline (static void _recv_setup(void)).

marked almost all these functions as static, please let me know if I missed one

keestux · 2019-02-03T21:54:12Z

sys/usb/usbus/usbus.c

+                usbus->state = USBUS_STATE_RESET;
+                usbus->addr = 0;
+                usbus->setup_state = USBUS_SETUPRQ_READY;
+                usbus->dev->driver->set(usbus->dev, USBOPT_ADDRESS, &usbus->addr, sizeof(uint8_t));


Is the sizeof correct? I'm asking because usbus->addr is a uint16_t.
And if your answer is no, then I would strongly suggest to change it to sizeof(usbus->addr)
And if your answer is yes, you should realize you have an endianess problem.

Not that it is breaking anything at the moment, but usbus->addr should be uint8_t 😕

Even if the addr should be uint8_t, it is safer to program this as sizeof(usbus->addr).

Agreed, fixed

keestux · 2019-02-03T21:58:18Z

sys/usb/usbus/usbus.c

+                }
+                break;
+            case USBDEV_EVENT_TR_FAIL:
+                if (ep->dir == USB_EP_DIR_OUT) {


Maybe add TODO marker? That is, if something needs to be done here. Otherwise drop the if

bergzand · 2019-02-13T18:35:14Z

rebased and reworked for the changes in #10915 and #9830.

Added some documentation (Always needs more docs, but maybe later). renamed and moved around some functions, mainly the usbus_control functions. Added a lot of static

bergzand · 2019-02-13T18:45:45Z

No longer WIP btw

keestux · 2019-02-13T19:54:47Z

sys/usb/usbus/usbus.c

+void usbus_init(usbus_t *usbus, usbdev_t *usbdev)
+{
+    /* TODO: Check if memset works/is necessary */
+    memset(usbus, 0, sizeof(usbus));


Shouldn't this be sizeof(*usbus) ?

Yes

Noticed it after I rebased and pushed everything 😢

Fixed

keestux · 2019-02-13T20:34:05Z

Is this the right place for this?

main(): This is RIOT! (Version: 2018.10-devel-544-g6bceee-rapper.fritz.box-usb-sodaq-one)
RIOT USB stack example application
All up, running the shell now
> 0x3797
*** RIOT kernel panic:
FAILED ASSERTION.

*** halted.

bergzand · 2019-02-13T20:42:32Z

Is this the right place for this?

Sure, no worries. If possible next time please add CFLAGS += -DDEBUG_ASSERT_VERBOSE to the makefile, then the line number where the assert failure occured is printed.

I must have forgotten to actually commit and push the change that modified the usbus::addr member to an uint8_t, thus causing an assertion failure in the usbdev peripheral.

keestux · 2019-02-13T21:14:21Z

Yes, much better now

bergzand · 2019-06-05T08:56:11Z

And rebased

bergzand · 2019-06-05T09:09:26Z

@kaspar030 Do you have any inspiration on how to fix the build failure? To be honest I really don't want to hardcode the pid.codes test PID in the example.

bergzand · 2019-06-05T10:09:05Z

Now depends on #11633 :'(

dylad · 2019-06-05T11:01:58Z

Please squash and rebase

bergzand · 2019-06-05T12:23:06Z

Please squash and rebase

Done!

dylad

Last minute error spotted.

dylad · 2019-06-05T12:46:53Z

For the record:
log from lsusb -vvv shows that VID/PID are inverted

Bus 001 Device 066: ID 0001:1209 Fry's Electronics 
Couldn't open device, some information will be missing
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0        64
  idVendor           0x0001 Fry's Electronics
  idProduct          0x1209 
  bcdDevice            0.00
  iManufacturer           3 
  iProduct                2 
  iSerial                 0 
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength            9
    bNumInterfaces          0
    bConfigurationValue     1
    iConfiguration          1 
    bmAttributes         0x80
      (Bus Powered)
    MaxPower              100mA

bergzand · 2019-06-05T12:55:18Z

log from lsusb -vvv shows that VID/PID are inverted

Aaah, thanks for testing, I switched around the VID/PID in the Makefile. Should be fixed now

dylad

RE-ACK
Retested lastest changes. We're all good.

dylad · 2019-06-05T13:07:17Z

Here we go !
Awesome job from @bergzand there ! You definitely deserve a medal :)

bergzand · 2019-06-05T13:07:51Z

🎉

@dylad, @keestux and @kaspar030 Thanks for reviewing!

emmanuelsearch · 2019-06-05T14:01:29Z

@bergzand yay!

kaspar030 · 2019-06-05T14:06:37Z

@bergzand niiiice!

brookstalley · 2019-06-05T14:44:56Z

@bergzand thank you so much. I have been checking this thread every morning the way some people check the lottery numbers.

bergzand added State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Jan 31, 2019

bergzand requested a review from dylad January 31, 2019 22:07

dylad reviewed Feb 1, 2019

View reviewed changes

keestux reviewed Feb 1, 2019

View reviewed changes

cpu/sam0_common/include/sam_usb.h Outdated

*/

/**

* @defgroup sys_uuid RFC 4122 compliant UUID's

Copy link

Contributor

keestux Feb 1, 2019

Choose a reason for hiding this comment

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

Copy-paste error?

keestux reviewed Feb 1, 2019

View reviewed changes

kaspar030 reviewed Feb 1, 2019

View reviewed changes

keestux reviewed Feb 1, 2019

View reviewed changes

bergzand force-pushed the pr/usb/usbus branch from f740cdb to e4ae27b Compare February 2, 2019 16:01

keestux reviewed Feb 3, 2019

View reviewed changes

bergzand force-pushed the pr/usb/usbus branch 2 times, most recently from 9e1a286 to 0ba41cc Compare February 13, 2019 18:13

bergzand removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Feb 13, 2019

keestux reviewed Feb 13, 2019

View reviewed changes

bergzand force-pushed the pr/usb/usbus branch from 4f195a5 to 9b0334d Compare February 25, 2019 19:21

bergzand force-pushed the pr/usb/usbus branch from f429d07 to dd2f09c Compare June 5, 2019 10:06

bergzand mentioned this pull request Jun 5, 2019

usbdev: fix usbopt argument for getter/setter wrappers #11633

Merged

bergzand added 2 commits June 5, 2019 14:22

usb: Add generic USB structs and functions

35af9b9

usbus: Initial work to a unified USB stack

74e0b5b

bergzand force-pushed the pr/usb/usbus branch 2 times, most recently from 061fa18 to 3d30886 Compare June 5, 2019 12:22

dylad added the Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines label Jun 5, 2019

dylad requested changes Jun 5, 2019

View reviewed changes

USBUS: add minimal working example

537add6

bergzand force-pushed the pr/usb/usbus branch from 3d30886 to 537add6 Compare June 5, 2019 12:51

dylad approved these changes Jun 5, 2019

View reviewed changes

dylad added the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label Jun 5, 2019

dylad merged commit e98376c into RIOT-OS:master Jun 5, 2019

keestux mentioned this pull request Jun 19, 2019

usbus: simplify adding entry to list #11726

Merged

PeterKietzmann mentioned this pull request Jul 23, 2019

USB Support #2284

Closed

USBUS: Initial work towards an USB stack #10916

USBUS: Initial work towards an USB stack #10916

Conversation

bergzand commented Jan 31, 2019 • edited Loading

Contribution description

Testing procedure

Issues/PRs references

Todo:

dylad commented Feb 1, 2019

bergzand commented Feb 1, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keestux left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bergzand commented Feb 2, 2019

dylad commented Feb 2, 2019

dylad commented Feb 2, 2019

keestux commented Feb 2, 2019

keestux commented Feb 2, 2019

bergzand commented Feb 2, 2019

keestux commented Feb 3, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bergzand commented Feb 13, 2019

bergzand commented Feb 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keestux commented Feb 13, 2019

bergzand commented Feb 13, 2019

keestux commented Feb 13, 2019

bergzand commented Jun 5, 2019

bergzand commented Jun 5, 2019

bergzand commented Jun 5, 2019

dylad commented Jun 5, 2019

bergzand commented Jun 5, 2019

dylad left a comment

Choose a reason for hiding this comment

dylad commented Jun 5, 2019

bergzand commented Jun 5, 2019

bergzand commented Jan 31, 2019 •

edited

Loading