cpu/atmega_common: add adc driver #7227

ghost · 2017-06-22T11:50:00Z

This PR is the reworked version of PR #6616 (Atmega adc support). It contains the ADC driver for Atmega-Common and Waspmote Pro board. The ADC driver covers now atmega328p, atmega2560 and atmega1281.

PeterKietzmann · 2017-06-22T12:24:11Z

@mali would you mind to close #6616 and help reviewing this PR instead? As you can see, it already contains your commits and your code. We currently have the capacity to take this one over, let's use it :-)!

@mali can you give a quick hint about the arduino pin map

aabadie

Quick review and found some inconsistencies.

aabadie · 2017-06-22T12:11:41Z

boards/waspmote-pro/include/periph_conf.h

@@ -104,6 +104,24 @@ extern "C" {
 #define MEGA_PRR            PRR0        /* Power Reduction Register */
 /** @} */

+/**
+* @brief ADC configuration


Should be @name

aabadie · 2017-06-22T12:11:53Z

boards/waspmote-pro/include/periph_conf.h

@@ -104,6 +104,24 @@ extern "C" {
 #define MEGA_PRR            PRR0        /* Power Reduction Register */
 /** @} */

+/**
+* @brief ADC configuration
+*


This line can be removed

aabadie · 2017-06-22T12:26:57Z

cpu/atmega_common/periph/adc.c

+ */
+
+/**
+ * @ingroup     driver_periph


should be drivers_periph

smlng · 2017-06-22T14:48:11Z

boards/arduino-atmega-common/include/arduino_pinmap.h

+#define A4                     4
+#define A5                     5
+#ifdef CPU_ATMEGA2560
+#define A6                     0


are the defines for A6 to A9 correct, 'cause they are the same as A0 to A3 and would reuse same ADC channels?

jnohlgard · 2017-06-23T05:12:08Z

boards/arduino-atmega-common/include/arduino_pinmap.h

@@ -156,6 +156,26 @@ extern "C" {
 #define ARDUINO_PIN_A15         ARDUINO_PIN_69
 #endif

+/* arduino ADC notation compatibility */
+#define A0                     0


Don't define macros with short names like this, it has the potential to create some very hard to track down problems later on when users try to use A0 etc as a variable name.

ghost · 2017-06-28T09:44:17Z

I have adapted this Pull Request to your comments. Please review the new commit.

mali · 2017-06-28T19:40:26Z

Looks good to me at first look, I should have time to test it between 5 and 11 July.

haukepetersen · 2017-06-29T12:47:19Z

boards/arduino-atmega-common/include/arduino_pinmap.h

+#define ADC12                    12
+#define ADC13                    13
+#define ADC14                    14
+#define ADC15                    15


I would prefer to stick with the existing naming see e.g. boards/arduino-due/arduino_pinmap.h. Also the ADC lines should be defined using the ADC_LINE(x) macro:

#define ARDUINO_A0 ADC_LINE(a) #define ARDUINO_A1 ADC_LINE(b) #define ARDUINO_A2 ADC_LINE(c) ...

Further this ifdef jungle looks pretty ugly -> why not define these things directly once for each specific board in the corresponding boards source tree?

I will remove the ifdef jungle, by removing the superfluous MCU. So I think, that the ADC defines can be remain in "boards/arduino-atmega-common/in clude/arduino_pinmap", because it includes already pin mapping of these MCU's.

haukepetersen · 2017-06-29T12:49:14Z

boards/arduino-atmega-common/include/periph_conf.h

+#endif
+#ifdef CPU_ATMEGA1281
+#define ADC_NUMOF       (8)
+#endif


this has nothing to do with the boards, but is CPU specific. So this belongs into the CPU code tree.

Yes, but all periphery of these MCU's (e.g. SPI, UART) is defined in "boards/arduino-atmega-common/include/periph_conf.h". So I think, that the ADC defines can be remain there?

Then this would have to be restructured everything?

haukepetersen · 2017-06-29T12:49:39Z

boards/waspmote-pro/include/periph_conf.h

+#define ADC_PIN_5           5
+#define ADC_PIN_6           6
+#define ADC_PIN_7           7
+/** @} */


same here, use ARDUINO_Ax ADC_LINE(x)

It is no Arduino board. So I will remove ADC channel configuration.

haukepetersen · 2017-06-29T12:52:02Z

sys/arduino/base.cpp

+int analogRead(int pin)
+{
+    adc_init((adc_t)pin);
+    return adc_sample((adc_t)pin, (adc_res_t)0);


don't think the simple cast of pin to adc_t will suffice here. IMHO this functions needs

check, if the given pin is a analog pin

map the analog pin number to a RIOT ADC_LINE, possibly something like ADC_LINE(pin - SOME_OFFSET?)

ghost · 2017-07-03T08:44:58Z

The Pull Request is adapted to additional comments.

haukepetersen · 2017-07-11T12:17:32Z

sys/arduino/base.cpp

+     * 0: Not initialized
+     * 1: Successfully initialized
+     */
+    static int adc_state[ADC_NUMOF];


for style reasons, I would prefer to define this variable outside of the function, its easier to read that way...

Also: IMHO it does not make sense to waste a lot of memory here by allocating a full int for each ADC line. How about a bitfield instead? Using a uint16_t should be ok, uint32_t would be pretty safe, or even better the type could be defined depending on the value ADC_NUMOF...

haukepetersen · 2017-07-11T12:22:51Z

sys/arduino/base.cpp

+
+    /* Initialization of given ADC channel */
+    if (adc_state[adc_pin] == 0) {
+        if (adc_init(ADC_LINE(adc_pin)) == 0) {


this is not correct: it is not guaranteed, that the RIOT ADC lines is mapped 'in order' to the Ardunio ADC pins (see boards/ardunio-due/include/arduino-pinmap.h as example... The solution here is the same as done for the Arduino digital pins: we need to define a mapping between Ardunio analog pin numbers to actual RIOT ADC lines (as in boards/arduino-due/include/arduino-board.h):

static const adc_t arduino_analog_pinmap[] = { ARDUINO_A0, .... };

As memo for a follow-up PR: we might as well generalize the .../arduino-board.h files to a central location and use ifdefs to create the actual pin mapping arrays...

The mapping of the RIOT ADC lines to the Arduino ADC pins is in the adc_init() funktion. Therefore, I only adapted the Arduino pin numbers to the ADC lines through a subtraction of ADC_PIN_OFFSET.

haukepetersen · 2017-07-11T12:24:24Z

found one more issue with the pin mapping

haukepetersen · 2017-07-13T11:10:50Z

it seems we have a misunderstanding here, the changes introduced with the last commit do not quite fix the issue at hand... I try to explain in the inline comments below.

haukepetersen · 2017-07-13T11:14:14Z

sys/arduino/base.cpp

+ */
+ static uint16_t adc_state;
+
+int analogRead(int arduino_pin)


to make sure we are on the same page: arduino ADC pins (the value of arduion_pin) are handled as unsigned integers starting from 0 for ARDUINO_A0 to x. To quote from here:

Syntax analogRead(pin) Parameters pin: the number of the analog input pin to read from (0 to 5 on most boards, 0 to 7 on the Mini and Nano, 0 to 15 on the Mega)

haukepetersen · 2017-07-13T11:15:57Z

sys/arduino/base.cpp

+    int adc_value;
+
+    /* Map the arduino pin number to the ADC line */
+    uint16_t adc_line_ = (uint16_t) (arduino_pin - ADC_PIN_OFFSET);


so given that ardunio_pin is a number from [0:x] with x being the available analog ports on that board, this line is broken when substracting ADC_PIN_OFFSET from such a value...

haukepetersen · 2017-07-13T11:17:43Z

boards/arduino-atmega-common/include/arduino_pinmap.h

@@ -155,7 +155,6 @@ extern "C" {
 #define ARDUINO_PIN_A14         ARDUINO_PIN_68
 #define ARDUINO_PIN_A15         ARDUINO_PIN_69
 #endif
-


you shoud re-add the ARDUINO_Ax defines!

haukepetersen · 2017-07-13T11:30:41Z

So to sketch the solution I had in mind:

in boards/x/include/arduino_pinmap.h:

re-add the #define ARDUINO_Ax ADC_LINE(x) defines

in boards/x/include/arduino_board.h:

add a static mapping of the available analog pins:

...
/**
 * @brief   Look-up table for analog Arduino pins
 */
static const adc_t arduino_analog_map[] = {
    ARDUINO_A0,
    ARDUINO_A1,
    ARDUINO_A2,
    ARDUINO_A3,
    ...
};

and finally in sys/arduino/base.cpp:

#define ANALOG_PIN_NUMOF     (sizeof(arduino_analog_map) / sizeof(arduino_analog_map[0]))

static uint16_t adc_state = 0;

int analogRead(int arduino_pin)  // remember, a value from [0 to number of analog Arduino pins]
{
    int adc_value;

    /* make sure the given pin is valid - TODO: better return error value?! */
    assert((arduino_pin >= 0) && (arduino_pin < ANALOG_PIN_NUMOF);

    /* Initialization of given ADC channel */
    if (!(adc_state & (1 << arduino_pin))) {
        if (adc_init(arduino_analog_map[arduino_pin] == 0)) {
            adc_state |= (1 << arduino_pin);
        }
        else {
            return -1; /* TODO: handle error as expected by ARDUNIO API */
        }
    }

    /* Read the ADC channel */
    adc_value = adc_sample(ADC_LINE(arduino_analog_map[arduino_pin]), ADC_RES_10BIT);

    return adc_value;
}

that should be it...

ghost · 2017-07-31T12:26:59Z

@haukepetersen okay to squash?

smlng

some comments, still need to test.

smlng · 2017-08-24T08:25:46Z

sys/arduino/base.cpp

+    /* Check the result of ADC */
+    if (adc_value == -2) {
+        /* The resolution is not valid */
+        return -2;


this if is not needed, as you return adc_value unchanged anyway.

Yes, you are right ;)

smlng · 2017-08-24T08:27:21Z

sys/arduino/base.cpp

+
+    /* Initialization of given ADC channel */
+    if (!(adc_line_state & (1 << arduino_pin))) {
+        if (adc_init(ADC_LINE(arduino_analog_map[arduino_pin])) == 0) {


IMHO simpler version:

if (adc_init(ADC_LINE(arduino_analog_map[arduino_pin])) != 0) { return -1; } /* The ADC channel is initialized */ adc_line_state |= 1 << arduino_pin;

Also: drop the ADC_LINE here! arduino_analog_map already consists of entries of type adc_t...

smlng · 2017-08-24T08:29:31Z

sys/arduino/base.cpp

+ * 0: Not initialized
+ * 1: Successfully initialized
+ */
+ static uint16_t adc_line_state = 0;


remove leading whitespace, btw. could use util function from bitfield.h here

smlng · 2017-08-24T08:30:19Z

sys/arduino/include/arduino.hpp

+ * @param[in] pin       pin to read
+ *
+ * @return a value between 0 to 1023 that is proportionnal
+ * to the voltage applied to the pin


according the above, it may also return -1 or -2 on error

smlng · 2017-08-24T08:32:52Z

cpu/atmega_common/periph/adc.c

+    prep();
+
+    /* Disable corresponding Digital input */
+    #if defined (CPU_ATMEGA328P) || defined (CPU_ATMEGA1281)


indention if of by one tab for all #if statements following

smlng · 2017-08-24T08:34:29Z

cpu/atmega_common/periph/adc.c

+        PORTC &= ~(1 << line);
+    #endif
+    #ifdef CPU_ATMEGA2560
+        if (line < 8) {


maybe move upward to previous matching if else simplifies preprocessor stuff a bit

smlng · 2017-08-24T08:36:23Z

cpu/atmega_common/periph/adc.c

+    ADCSRA |= (1 << ADSC);
+
+    /* Wait until the conversion is complete */
+    while(ADCSRA & (1 << ADSC));


coding style (space before parentheses and braces instead of semi-colon) should be while (ADCSRA & (1 << ADSC)) {}

smlng · 2017-08-24T08:36:46Z

cpu/atmega_common/periph/adc.c

+        ADMUX |= line;
+    #endif
+    #ifdef CPU_ATMEGA2560
+        if(line < 8) {


coding style, missing space after if

haukepetersen · 2017-08-24T12:32:03Z

boards/arduino-atmega-common/include/arduino_pinmap.h

+#define ARDUINO_A2              ADC_LINE(2)
+#define ARDUINO_A3              ADC_LINE(3)
+#define ARDUINO_A4              ADC_LINE(4)
+#define ARDUINO_A5              ADC_LINE(5)


Code duplication here. You can omit the #ifdef CPU_ATMEGA328P above and define A0-A5 for all boards and then you optionally add A6-A15 for the mega2560...

Atmega328p (arduino nano) has also lines A6 and A7, please move the following #ifdef two lines below

haukepetersen · 2017-08-24T13:36:55Z

boards/arduino-atmega-common/include/periph_conf.h

+#ifdef CPU_ATMEGA2560
+#define ADC_NUMOF       (16)
+#define ADC_PIN_OFFSET  54
+#endif


#if defined(CPU_ATMEGA328P) #define ADC_NUMOF (5) #define ADC_PIN_OFFSET 14 #elif defined(CPU_ATMEGA2560) #define ADC_NUMOF (16) #define ADC_PIN_OFFSET 54 #endif

would be a little more compact...

haukepetersen · 2017-08-24T13:36:57Z

boards/arduino-atmega-common/include/periph_conf.h

+#ifdef CPU_ATMEGA2560
+#define ADC_NUMOF       (16)
+#define ADC_PIN_OFFSET  54
+#endif


#if defined(CPU_ATMEGA328P) #define ADC_NUMOF (5) #define ADC_PIN_OFFSET 14 #elif defined(CPU_ATMEGA2560) #define ADC_NUMOF (16) #define ADC_PIN_OFFSET 54 #endif

would be a little more compact...

And also: do I see it correctly, that ADC_PIN_OFFSET is not used anywhere anymore?!

Yes, you are right ;)

haukepetersen · 2017-08-24T13:39:06Z

cpu/atmega_common/periph/adc.c

+        if(line < 8)
+            DIDR0 |= (1 << line);
+        else
+            DIDR2 |= (1 << (line-8));


spaces missing: (line - 8)

haukepetersen · 2017-08-24T13:40:39Z

cpu/atmega_common/periph/adc.c

+    /* check if the line is valid */
+    if (line >= ADC_NUMOF) {
+        return -1;
+    }


the line should be checked using assert here.

@haukepetersen actually not, the documentation states "@return -1 on invalid ADC line"

true, never mind.

haukepetersen · 2017-08-24T13:40:59Z

cpu/atmega_common/periph/adc.c

+
+    /* check if resolution is applicable */
+    if (res != ADC_RES_10BIT) {
+        return -2;


As the API doc states: @return -1 if resolution is not applicable...

haukepetersen · 2017-08-24T13:42:10Z

cpu/atmega_common/periph/adc.c

+        ADMUX &= 0xf0;
+        ADMUX |= line;
+    #endif
+    #ifdef CPU_ATMEGA2560


why not use a straight forward #elif defined(CPU_ATMEGA2560) here? This would be IMHO a little easier to read...

haukepetersen · 2017-08-24T13:44:56Z

sys/arduino/base.cpp

+
+    /* Initialization of given ADC channel */
+    if (!(adc_line_state & (1 << arduino_pin))) {
+        if (adc_init(ADC_LINE(arduino_analog_map[arduino_pin])) == 0) {


Also: drop the ADC_LINE here! arduino_analog_map already consists of entries of type adc_t...

haukepetersen · 2017-08-24T13:45:06Z

sys/arduino/base.cpp

+    }
+
+    /* Read the ADC channel */
+    adc_value = adc_sample(ADC_LINE(arduino_analog_map[arduino_pin]), ADC_RES_10BIT);


drop ADC_LINE here as well

smlng · 2017-08-30T11:35:51Z

Just tested with mega2560, and it works fine. Please address latest comments by @haukepetersen and me, otherwise pre-ACK.

lebrush

I think this could be a bit more flexible. Init function should do nothing but initialise pin and disable the digital buffer (DIR0 and DIR1). Adc sample should do the rest. Implementing it using the interrupt rather than a busy wait could be useful too :-)

lebrush · 2017-08-30T17:13:34Z

boards/arduino-atmega-common/include/arduino_pinmap.h

+#define ARDUINO_A2              ADC_LINE(2)
+#define ARDUINO_A3              ADC_LINE(3)
+#define ARDUINO_A4              ADC_LINE(4)
+#define ARDUINO_A5              ADC_LINE(5)


Atmega328p (arduino nano) has also lines A6 and A7, please move the following #ifdef two lines below

lebrush · 2017-08-30T17:14:02Z

boards/arduino-atmega-common/include/periph_conf.h

+ * @{
+ */
+#if defined (CPU_ATMEGA328P)
+#define ADC_NUMOF       (5)


I suggest to set it to 8. For a project I remapped the software interrupt to another pin since it's easily doable. I also think the 1281 does not have lines over 7, maybe you can turn the defines around to simplify the code

#ifdef 2560 16 #else 8 #endif

@smlng what do you think about it?

considering #7528, plus the fact that the uno has 6 ADC while the nano/mini have 8 ADC but all atmega328p based, it might be better to move this into a board specific periph_conf. h instead anyway. So @lebrush is right, i.e., setting ADC_NUMOF to 5 for 328p is not correct for all boards.

lebrush · 2017-08-30T17:17:10Z

cpu/atmega_common/periph/adc.c

+    /* check if the line is valid */
+    if (line >= ADC_NUMOF) {
+        return -1;
+    }


@haukepetersen actually not, the documentation states "@return -1 on invalid ADC line"

lebrush · 2017-08-30T17:20:23Z

cpu/atmega_common/periph/adc.c

+    int sample = 0;
+
+    /* check if resolution is applicable */
+    assert(res == ADC_RES_10BIT);


This should be a return. Documentation states:
@return -1 if resolution is not applicable

lebrush · 2017-08-30T17:24:04Z

cpu/atmega_common/periph/adc.c

+        return -1;
+    }
+
+    prep();


ADC doesn't need to be enabled here

But it must be locked

lebrush · 2017-08-30T17:31:17Z

cpu/atmega_common/periph/adc.c

+        PORTF &= ~(1 << line);
+    #endif
+
+    /* set clock prescaler to get the maximal possible ADC clock value */


Idea: since prescalers just depends in the coreclock, can be set by the pre-processor

/* sampling freq must be between 200kHz and 50kHz */ #if CLOCK_CORECLOCK > 12800000 #define PRESCALER 0x07 /* 128 */ #elif CLOCK_CORECLOCK > 6400000 #define PRESCALER 0x06 /* 64 */ #elif CLOCK_CORECLOCK > 3200000 #define PRESCALER 0x05 /* 32 */ #elif CLOCK_CORECLOCK > 1600000 #define PRESCALER 0x04 /* 16 */ #elif CLOCK_CORECLOCK > 800000 #define PRESCALER 0x03 /* 8 */ #elif CLOCK_CORECLOCK > 100000 #define PRESCALER 0x01 /* 2 */ #else #error CLOCK must be at least 100kHz to use ADC #endif

in #7502 its the other way around, in general I'd say the compiler should optimise that out anyway. But to be sure, you could make a size comparison.

However, I would leave as is for now.

lebrush · 2017-08-30T17:36:30Z

cpu/atmega_common/periph/adc.c

+    ADCSRA |= clk_div;
+
+    /* Ref Voltage is Vcc(5V) */
+    ADMUX |= (1 << REFS0);


I wouldn't force it to 5V, it' might be interesting to have VREF as reference voltage. Plus in the 1281/2560 it can be set to differential measurements which might be quite useful.

I suggest introduce a configuration struct, which allows to configure it for each channel. Then set the reference when doing the triggering the sample.

typedef struct { gpio_t pin; adc_ref_t ref; uint8_t ch; } adc_conf_t; ... static const adc_conf_t adc_conf[] = { { GPIO_PIN(ADC_PORT, 0), ADC_REF_AVCC, 0 }, { GPIO_PIN(ADC_PORT, 1), ADC_REF_AVCC, 1 }, { GPIO_PIN(ADC_PORT, 2), ADC_REF_AVCC, 2 }, ... }

This also simplifies the adc_init and adc_sample code.

In general a good idea, specifically to have the reference voltage configurable. What I don't like is the duplication in pin and ch because they have a direct dependency and ADC_PORT is fixed for all anyway.

but maybe we should get general ADC support first and fix this later on. A first step could also be to make the ref voltage configurable via define macro (though it applies to all pins then).

lebrush · 2017-08-30T17:37:07Z

cpu/atmega_common/periph/adc.c

+    /* check if resolution is applicable */
+    assert(res == ADC_RES_10BIT);
+
+    prep();


it has to be enabled after setting the other registers otherwise some of the registers of the 328p can't be written

lebrush · 2017-08-30T17:40:32Z

boards/arduino-atmega-common/include/arduino_board.h

+    ARDUINO_A3,
+    ARDUINO_A4,
+    ARDUINO_A5,
+#ifdef CPU_ATMEGA2560


move down two lines (see my comments below)

lebrush · 2017-08-30T17:48:01Z

cpu/atmega_common/periph/adc.c

+    while(ADCSRA & (1 << ADSC)) {}
+
+    /* Get conversion result */
+    sample = ADC;


Result is 10 bits, you need to read two registers (in the correct order to get the result)

Both registers (ADCL, ADCH) are read out via ADC register

smlng · 2017-09-28T13:03:34Z

@dnahm please rebase, address comments by @lebrush and squash, too.

smlng · 2017-11-07T14:03:33Z

@PeterKietzmann how do we proceed here? I could overtake this one, shouldn't be that much work - still need some to review and ack afterwards :)

PeterKietzmann · 2017-11-07T14:08:41Z

@smlng please do so! Offline @dnahm agreed on closing this PR. However, please base your rework on his commits. I can undertake the testing part.

smlng · 2017-11-07T20:34:27Z

closing as discussed, replaced by #7954

ghost mentioned this pull request Jun 22, 2017

Atmega adc support #6616

Closed

PeterKietzmann requested a review from mali June 22, 2017 12:18

PeterKietzmann added the Platform: AVR Platform: This PR/issue effects AVR-based platforms label Jun 22, 2017

PeterKietzmann assigned PeterKietzmann and smlng Jun 22, 2017

PeterKietzmann added the Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation label Jun 22, 2017

aabadie requested changes Jun 22, 2017

View reviewed changes

aabadie added this to the Release 2017.07 milestone Jun 22, 2017

smlng reviewed Jun 22, 2017

View reviewed changes

jnohlgard requested changes Jun 23, 2017

View reviewed changes

haukepetersen requested changes Jun 29, 2017

View reviewed changes

aabadie modified the milestones: Release 2017.07, Release 2017.10 Jun 30, 2017

haukepetersen requested changes Jul 11, 2017

View reviewed changes

haukepetersen reviewed Jul 13, 2017

View reviewed changes

smlng suggested changes Aug 24, 2017

View reviewed changes

haukepetersen requested changes Aug 24, 2017

View reviewed changes

lebrush suggested changes Aug 30, 2017

View reviewed changes

smlng added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Sep 1, 2017

smlng added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 28, 2017

cpu/atmega_common: add adc driver

dbc85df

aabadie mentioned this pull request Oct 5, 2017

added ADC driver for atmega328p #7676

Closed

smlng mentioned this pull request Oct 6, 2017

boards/arduino-nano: port arduino nano board #7540

Closed

smlng mentioned this pull request Nov 7, 2017

cpu, atmega_common: add periph/adc #7954

Merged

smlng closed this Nov 7, 2017

miri64 added the CI: needs squashing label Oct 5, 2018

cpu/atmega_common: add adc driver #7227

cpu/atmega_common: add adc driver #7227

Uh oh!

Conversation

ghost commented Jun 22, 2017 • edited by aabadie Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PeterKietzmann commented Jun 22, 2017

Uh oh!

aabadie left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Jun 28, 2017

Uh oh!

mali commented Jun 28, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Jul 3, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haukepetersen commented Jul 11, 2017

Uh oh!

haukepetersen commented Jul 13, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haukepetersen commented Jul 13, 2017

Uh oh!

ghost commented Jul 31, 2017

Uh oh!

smlng left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

ghost commented Jun 22, 2017 •

edited by aabadie

Loading