sys/senml: add SenML modules #16384

silkeh · 2021-04-24T13:57:46Z

Contribution description

Add a basic SenML module with support for:

Encoding SenML values as CBOR using NanoCBOR.
Converting from Phydat to SenML.
Reading and encoding SAUL sensors.

Some effort is made to convert units from those in Phydat to those supported by SenML, which results in some code that should probably not be part of a SenML implementation (including calculation of Unix timestamps).

Testing procedure

Includes two test cases and an example. The senml_saul test isn't useful at this point, but I couldn't find if there are some mocked sensors already.

Issues/PRs references

Depends on bergzand/NanoCBOR#50

sys/senml/phydat.c

fjmolinas

In general, this looks nice. I'm not sure about the senml_saul module though. I'm not sure many users will need it since it reads all saul_reg_t by default, it seems like something that would be better fitted to be in the examples.

I think what would make more sense is having a:

void senml_saulreg_encode(void *ctx, saul_reg_t *dev);
ssize_t senml_saulregs_encode(coid *ctx, saul_reg_t **dev, uint8_t num);

Where the second would be an array of saul_regs_t. I think in most cases the user would know how many sensors it will encode and therefore a definite array is a better solution, a convenience wrapper could be provided as well, so something like:

ssize_t senml_saulregs_encode_cbor(uint8_t *buf, size_t len, saul_reg_t **dev, uint8_t num)
{
    nanocbor_encoder_t enc;

    nanocbor_encoder_init(&enc, buf, len);
    nanocbor_fmt_array(&enc, num);
    for (uint8_t i = 0; i < num; i++) {
        senml_saulreg_encode(&enc, *dev++);
    }
    return nanocbor_encoded_len(&enc);
}

fjmolinas · 2021-05-07T14:49:03Z

makefiles/pseudomodules.inc.mk

@@ -125,6 +125,7 @@ PSEUDOMODULES += saul_pwm
 PSEUDOMODULES += scanf_float
 PSEUDOMODULES += sched_cb
 PSEUDOMODULES += semtech_loramac_rx
+PSEUDOMODULES += senml_%


Its better to use explicit modules, it at least keeps a clear list of the modules that are expected or not.

fjmolinas · 2021-05-18T09:22:55Z

sys/include/senml.h

+    senml_unit_t base_unit;         /**< Base Unit */
+    senml_numeric_t base_value;     /**< Base Value */
+    senml_numeric_t base_sum;       /**< Base Sum */
+    uint64_t base_version;          /**< Base Version */


Does it make sense to have compile-time configuration remove some of the optional fields and save some RAM?, specifically base_version, update_time, sum, which will be used more rarely.

fjmolinas · 2021-05-18T09:23:15Z

sys/include/senml.h

+ * @param v Value to encode.
+ * @return Numeric value containing the given value.
+ */
+inline senml_numeric_t senml_float(float v)


Suggested change

inline senml_numeric_t senml_float(float v)

static inline senml_numeric_t senml_float(float v)

And for other cases below.

fjmolinas · 2021-05-18T09:24:56Z

sys/include/senml.h

+ * @param v Value to encode.
+ * @return Numeric value containing the given value.
+ */
+inline senml_numeric_t senml_float(float v)


Could we add static initializers as well? Some of the data structures are quite heavy (150bytes) and could be statically allocated to save stack space in many cases.

sys/senml/senml.c

pkg/nanocbor/Makefile

fjmolinas · 2021-06-11T09:25:49Z

This is no longer waiting. Can you rebase @silkeh

Citrullin · 2021-08-25T15:52:54Z

In general, this looks nice. I'm not sure about the senml_saul module though. I'm not sure many users will need it since it reads all saul_reg_t by default, it seems like something that would be better fitted to be in the examples.

I think what would make more sense is having a:
void senml_saulreg_encode(void *ctx, saul_reg_t *dev);
ssize_t senml_saulregs_encode(coid *ctx, saul_reg_t **dev, uint8_t num);
Where the second would be an array of saul_regs_t. I think in most cases the user would know how many sensors it will encode and therefore a definite array is a better solution, a convenience wrapper could be provided as well, so something like:
ssize_t senml_saulregs_encode_cbor(uint8_t *buf, size_t len, saul_reg_t **dev, uint8_t num)
{
    nanocbor_encoder_t enc;

    nanocbor_encoder_init(&enc, buf, len);
    nanocbor_fmt_array(&enc, num);
    for (uint8_t i = 0; i < num; i++) {
        senml_saulreg_encode(&enc, *dev++);
    }
    return nanocbor_encoded_len(&enc);
}

I agree. I would also like to see an API for individual devices. This would be very helpful.

silkeh · 2021-09-01T11:41:01Z

In general, this looks nice. I'm not sure about the senml_saul module though. I'm not sure many users will need it since it reads all saul_reg_t by default, it seems like something that would be better fitted to be in the examples.

I think what would make more sense is having a:
void senml_saulreg_encode(void *ctx, saul_reg_t *dev);
ssize_t senml_saulregs_encode(coid *ctx, saul_reg_t **dev, uint8_t num);
Where the second would be an array of saul_regs_t. I think in most cases the user would know how many sensors it will encode and therefore a definite array is a better solution, a convenience wrapper could be provided as well, so something like:
ssize_t senml_saulregs_encode_cbor(uint8_t *buf, size_t len, saul_reg_t **dev, uint8_t num)
{
    nanocbor_encoder_t enc;

    nanocbor_encoder_init(&enc, buf, len);
    nanocbor_fmt_array(&enc, num);
    for (uint8_t i = 0; i < num; i++) {
        senml_saulreg_encode(&enc, *dev++);
    }
    return nanocbor_encoded_len(&enc);
}

SAUL is structured in such a way that the dimensions of sensor values are entirely unknowable until you actually read it. I think it can even change on every read (theoretically). This means that the number of sensors and the number of SenML values have a ratio of somewhere between 1 and PHYDAT_DIM, and we cannot determine beforehand how many SenML values will be returned.

As for passing a list with a number of sensors: is it common to have a list in such a way? Why not just pass a saul_reg_t *dev, as that's a linked list already?

Taking the above into account, I have split the implementation in two functions:

int senml_saul_reg_encode_cbor(nanocbor_encoder_t *enc, saul_reg_t *dev);
size_t senml_saul_encode_cbor(uint8_t *buf, size_t len, saul_reg_t *reg);

The first function encodes all dimensions of a SAUL sensor (and returns the number of dimensions encoded), and the second one is encodes all functions in a registry. The second one is extremely simple now, and I think it would be fine to move that to the example if so desired.

Encoding all the temperature values would then look something like this:

nanocbor_encoder_t enc;

nanocbor_encoder_init(&enc, buf, len);
nanocbor_fmt_array_indefinite(&enc);

saul_reg_t *dev = saul_reg;
while (dev) {
    if(dev->driver->type = SAUL_SENSE_TEMP) {
        senml_saul_reg_encode_cbor(&enc, dev);
    }
    dev = dev->next;
}

nanocbor_fmt_end_indefinite(&enc);

Citrullin · 2021-09-22T16:01:25Z

SAUL is structured in such a way that the dimensions of sensor values are entirely unknowable until you actually read it. I think it can even change on every read (theoretically). This means that the number of sensors and the number of SenML values have a ratio of somewhere between 1 and PHYDAT_DIM, and we cannot determine beforehand how many SenML values will be returned.

As for passing a list with a number of sensors: is it common to have a list in such a way? Why not just pass a saul_reg_t *dev, as that's a linked list already?

I think a point you forget is that a user can also create a custom linked list of saul_reg_t. Additionally to the default SAUL register. This may be even needed for certain use-cases. Or, like in my case, I need to individually encode each sensor.
I don't want to have one endpoint with all sensors. I need them under seperated endpoints.

Taking the above into account, I have split the implementation in two functions:
int senml_saul_reg_encode_cbor(nanocbor_encoder_t *enc, saul_reg_t *dev);
size_t senml_saul_encode_cbor(uint8_t *buf, size_t len, saul_reg_t *reg);
The first function encodes all dimensions of a SAUL sensor (and returns the number of dimensions encoded), and the second one is encodes all functions in a registry. The second one is extremely simple now, and I think it would be fine to move that to the example if so desired.

For the second function: Why not having nanocbor_encoder_t here as well. I am not familiar with nanocbor. Can't you reuse the encoder instead of initializing another one for each device you may encode?

silkeh · 2021-09-22T17:29:11Z

SAUL is structured in such a way that the dimensions of sensor values are entirely unknowable until you actually read it. I think it can even change on every read (theoretically). This means that the number of sensors and the number of SenML values have a ratio of somewhere between 1 and PHYDAT_DIM, and we cannot determine beforehand how many SenML values will be returned.
As for passing a list with a number of sensors: is it common to have a list in such a way? Why not just pass a saul_reg_t *dev, as that's a linked list already?

I think a point you forget is that a user can also create a custom linked list of saul_reg_t. Additionally to the default SAUL register. This may be even needed for certain use-cases. Or, like in my case, I need to individually encode each sensor.
I don't want to have one endpoint with all sensors. I need them under seperated endpoints.

So why not create a custom registry instead of a linked list? The current implementation allows you to pass any saul_reg_t, including custom ones. For actually custom data structures I think it's fine to have to do a bit more effort. Especially because (depending on the use case), you can optimize a bit: for example, the following would encode a list of saul_reg_t sensors (with known dimensions):

nanocbor_encoder_t enc;
nanocbor_encoder_init(&enc, buf, len);
nanocbor_fmt_array(&enc, sensor_count);
for (int i = 0; i < sensor_count; i++) {
    senml_saul_reg_encode_cbor(&enc, sensors[i]);
}

return nanocbor_encoded_len(&enc)

Taking the above into account, I have split the implementation in two functions:
int senml_saul_reg_encode_cbor(nanocbor_encoder_t *enc, saul_reg_t *dev);
size_t senml_saul_encode_cbor(uint8_t *buf, size_t len, saul_reg_t *reg);
The first function encodes all dimensions of a SAUL sensor (and returns the number of dimensions encoded), and the second one is encodes all functions in a registry. The second one is extremely simple now, and I think it would be fine to move that to the example if so desired.
For the second function: Why not having nanocbor_encoder_t here as well. I am not familiar with nanocbor. Can't you reuse the encoder instead of initializing another one for each device you may encode?

Sure, but just use something like the example above. The code is sufficiently composable to do whatever you like.

kaspar030 · 2022-02-01T10:14:10Z

I'm testing tests/senml_cbor on z1. it reboots unless increasing stack size. I tried CFLAGS += -DTHREAD_STACKSIZE_DEFAULT=1536, that makes it pass.

fjmolinas · 2022-02-01T10:24:09Z

I'm testing tests/senml_cbor on z1. it reboots unless increasing stack size. I tried CFLAGS += -DTHREAD_STACKSIZE_DEFAULT=1536, that makes it pass.

Nice then its only a stack issue

fjmolinas · 2022-02-01T11:14:33Z

CFLAGS += -DTHREAD_STACKSIZE_DEFAULT=1536

Did the same thing or arduino-mega2560, test fails because encoded length is not as expected, form looking of the encoded payload the double is off:

2022-02-01 12:09:25,675 # START
2022-02-01 12:09:25,745 # main(): This is RIOT! (Version: 2022.04-devel-76-g35245-pr-16384)
2022-02-01 12:09:25,749 # .
2022-02-01 12:09:25,827 # senml_tests.test_senml_encode (tests/senml_cbor/main.c 107) exp 188 was 184
2022-02-01 12:09:25,827 # 
2022-02-01 12:09:25,847 # run 1 failures 1

[{-2: "CBOR test", -3: 1619264720, 1: "m", 7: 120, 2: 61.5}, {6: 1, 2: 2.46379138708083e-248}, 61, {6: 3, 2: 61}, {6: 4, 2: 4([-1, 615])}, {6: 5, 4: true}, {6: 6, 3: "RIOT OS"}, {6: 7, 8: h'00010203'}, {1: "kg", 5: 61}]

fjmolinas · 2022-02-01T11:23:45Z

So this patch was enough to fix compilation issues with avr:

diff --git a/tests/senml_cbor/Makefile b/tests/senml_cbor/Makefile
index 971ce780ee..abac3c6b6a 100644
--- a/tests/senml_cbor/Makefile
+++ b/tests/senml_cbor/Makefile
@@ -8,4 +8,6 @@ CFLAGS += -DCONFIG_SENML_ATTR_SUM
 CFLAGS += -DCONFIG_SENML_ATTR_VERSION
 CFLAGS += -DCONFIG_SENML_ATTR_UPDATE_TIME
 
+CFLAGS += -DTHREAD_STACKSIZE_DEFAULT=1536
+
 include $(RIOTBASE)/Makefile.include
diff --git a/tests/senml_phydat/Makefile b/tests/senml_phydat/Makefile
index 6c14df0faa..452d330fda 100644
--- a/tests/senml_phydat/Makefile
+++ b/tests/senml_phydat/Makefile
@@ -2,5 +2,6 @@ include ../Makefile.tests_common
 
 USEMODULE += senml_phydat
 USEMODULE += embunit
+USEMODULE += printf_float
 
 include $(RIOTBASE)/Makefile.include
diff --git a/tests/senml_phydat/main.c b/tests/senml_phydat/main.c
index 6cc295d96f..76b38bad66 100644
--- a/tests/senml_phydat/main.c
+++ b/tests/senml_phydat/main.c
@@ -28,6 +28,10 @@
 #define ENABLE_DEBUG (0)
 #include "debug.h"
 
+#ifndef PRIi64
+#define PRIi64 "lli"
+#endif
+
 typedef struct {
     phydat_t phydat;
     senml_value_t senml;

But still both avr and msp430 are failing with rounding floats:

avr

2022-02-01 12:21:51,830 # main(): This is RIOT! (Version: 2022.04-devel-76-g35245-pr-16384)
2022-02-01 12:21:51,834 # ...
2022-02-01 12:21:51,928 # senml_tests.test_phydat_to_senml_float (tests/senml_phydat/main.c 248) exp -10752 was -10753
2022-02-01 12:21:51,932 # .
2022-02-01 12:21:51,949 # run 4 failures 1

msp430

2022-02-01 12:22:31,503 # START
2022-02-01 12:22:31,511 # main(): This is RIOT! (Version: 2022.04-devel-76-g35245-pr-16384)
2022-02-01 12:22:31,673 # ...
2022-02-01 12:22:31,685 # senml_tests.test_phydat_to_senml_float (tests/senml_phydat/main.c 247) exp 25537 was 0
2022-02-01 12:22:31,685 # .
2022-02-01 12:22:31,686 # run 4 failures 1

What test fails depends on what round function is used (at least for me locally), maybe we should skip that test case for some platforms.

fjmolinas · 2022-02-01T11:25:28Z

CFLAGS += -DTHREAD_STACKSIZE_DEFAULT=1536

Did the same thing or arduino-mega2560, test fails because encoded length is not as expected, form looking of the encoded payload the double is off:

2022-02-01 12:09:25,675 # START
2022-02-01 12:09:25,745 # main(): This is RIOT! (Version: 2022.04-devel-76-g35245-pr-16384)
2022-02-01 12:09:25,749 # .
2022-02-01 12:09:25,827 # senml_tests.test_senml_encode (tests/senml_cbor/main.c 107) exp 188 was 184
2022-02-01 12:09:25,827 # 
2022-02-01 12:09:25,847 # run 1 failures 1

[{-2: "CBOR test", -3: 1619264720, 1: "m", 7: 120, 2: 61.5}, {6: 1, 2: 2.46379138708083e-248}, 61, {6: 3, 2: 61}, {6: 4, 2: 4([-1, 615])}, {6: 5, 4: true}, {6: 6, 3: "RIOT OS"}, {6: 7, 8: h'00010203'}, {1: "kg", 5: 61}]

This could be a nanocbor issue with AVR since the nanocbor_fmt_double is not tested, maybe @bergzand can say more

tests/senml_phydat/main.c

silkeh · 2022-02-09T13:55:02Z

But still both avr and msp430 are failing with rounding floats:

Maybe we should only run the test on platforms with known predictable rounding, eg:

diff --git a/tests/senml_phydat/main.c b/tests/senml_phydat/main.c
index 76b38bad66..02209fe596 100644
--- a/tests/senml_phydat/main.c
+++ b/tests/senml_phydat/main.c
@@ -278,7 +278,10 @@ Test *tests_senml(void)
     EMB_UNIT_TESTFIXTURES(fixtures) {
         new_TestFixture(test_phydat_date_to_senml),
         new_TestFixture(test_phydat_time_to_senml),
+/* Only run this test on CPUs with predictable rounding */
+#if defined(CPU_NATIVE) || defined(CPU_STM32)
         new_TestFixture(test_phydat_to_senml_float),
+# endif
         new_TestFixture(test_phydat_to_senml_decimal),
     };
     EMB_UNIT_TESTCALLER(senml_tests, NULL, NULL, fixtures);

If we really want to test if floats/rounding works predictably/reliably on all platforms there should probably be a different test for that.

fjmolinas · 2022-02-09T14:05:18Z

Maybe we should only run the test on platforms with known predictable rounding, eg:

I would rather go the other way around and not enable them for MSP430 and AVR. But I agree this is not an issue with this PR. For me currently, the only thing I want to figure out is why nanocbor_fmt_double is failing on AVR, no need to fix it here but just figure out it's not an issue with this PRs code.

silkeh · 2022-02-09T14:25:03Z

I would rather go the other way around and not enable them for MSP430 and AVR.

That's fine as well, although I'm having a bit of trouble finding what I should put in the # if !defined() statement to exclude msp430 and avr

fjmolinas · 2022-02-09T14:41:40Z

I would rather go the other way around and not enable them for MSP430 and AVR.

That's fine as well, although I'm having a bit of trouble finding what I should put in the # if !defined() statement to exclude msp430 and avr

For avr there is #if defined(__AVR__) for msp430 there is defined(CPU_MSP430FXYZ), not sure if there is another one, but those should do here,

silkeh · 2022-02-09T16:18:46Z

I have pushed three fixup commits:

Patch from @fjmolinas earlier.
Removal of all date/time related stuff as requested by @chrysn.
Exclusion of float tests for msp430 and AVR.

Please take a look and feel free to squash.

fjmolinas · 2022-02-11T08:26:06Z

So it seems there is indeed an issue with nanocbor_fmt_double on AVR, so unrelated to this PR.

fjmolinas · 2022-02-11T08:26:26Z

I pushed a couple of nitpicks

fjmolinas · 2022-02-11T08:26:58Z

Rebased

fjmolinas · 2022-02-11T08:27:11Z

Squashed

fjmolinas · 2022-02-11T08:28:00Z

@chrysn Are you ok with getting this one in and addressing SenML/PhyDat unit issues separately?

chrysn · 2022-02-11T08:31:59Z

Yes, either sequence is OK, and this PR appears to be quite ready.

fjmolinas

ACK, there are still some pending issues with:

issues with handling of doubles on AVR, but this seems to come from either nanocbor or the CPU itself
to limit builds jobs I would move all tests to a single application, this can be done in a follow-up, I have a branch with those somewhere (or a patch in the PR)

Other than this LGTM, I've tested on different architectures and BOARDs and except for the above mentioned issues and rounding differences on floats between platforms it all looks OK.

fjmolinas · 2022-02-11T11:26:41Z

Force pushed missing Kconfig in test and some blacklisting.

Add a basic SenML module and submodules with support for: - Encoding SenML values as CBOR using NanoCBOR. - Converting from Phydat to SenML. - Reading and encoding SAUL sensors.

fjmolinas · 2022-02-11T13:46:37Z

Wuhuuu! Thanks for this one @silkeh and thanks for sticking through!

silkeh · 2022-02-12T09:27:40Z

Yay! Thanks for all the help everyone!

silkeh requested review from aabadie, cgundogan, fjmolinas, jia200x, leandrolanzieri, MichelRottleuthner, miri64 and smlng as code owners April 24, 2021 13:57

jeandudey added Area: sys Area: System Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Apr 25, 2021

kfessel reviewed Apr 27, 2021

View reviewed changes

sys/senml/phydat.c Outdated Show resolved Hide resolved

jeandudey added the State: waiting for other PR State: The PR requires another PR to be merged first label May 14, 2021

fjmolinas reviewed May 18, 2021

View reviewed changes

bergzand reviewed Jun 8, 2021

View reviewed changes

pkg/nanocbor/Makefile Outdated Show resolved Hide resolved

fjmolinas removed the State: waiting for other PR State: The PR requires another PR to be merged first label Jun 11, 2021

MrKevinWeiss added this to the Release 2021.07 milestone Jun 22, 2021

MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021

fjmolinas added this to the Release 2022.01 milestone Dec 3, 2021

bergzand reviewed Feb 7, 2022

View reviewed changes

tests/senml_phydat/main.c Show resolved Hide resolved

fjmolinas force-pushed the pr/senml-module branch from 3f2d6aa to 35de03a Compare February 11, 2022 08:26

fjmolinas force-pushed the pr/senml-module branch from 35de03a to 512a1c5 Compare February 11, 2022 08:27

fjmolinas approved these changes Feb 11, 2022

View reviewed changes

fjmolinas force-pushed the pr/senml-module branch from 512a1c5 to 7db5992 Compare February 11, 2022 11:24

fjmolinas enabled auto-merge February 11, 2022 11:25

sys/senml: add SenML modules

9d61bdb

Add a basic SenML module and submodules with support for: - Encoding SenML values as CBOR using NanoCBOR. - Converting from Phydat to SenML. - Reading and encoding SAUL sensors.

fjmolinas force-pushed the pr/senml-module branch from 7db5992 to 9d61bdb Compare February 11, 2022 11:38

fjmolinas merged commit e6024c7 into RIOT-OS:master Feb 11, 2022

fjmolinas mentioned this pull request Feb 11, 2022

tests/senml: do no include saul_default #17641

Merged

silkeh mentioned this pull request Jan 1, 2023

sys/phydat: add functions for Unix time conversion to phydat #19080

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sys/senml: add SenML modules #16384

sys/senml: add SenML modules #16384

silkeh commented Apr 24, 2021 •

edited

Loading

fjmolinas left a comment

fjmolinas May 7, 2021

fjmolinas May 18, 2021

fjmolinas May 18, 2021

fjmolinas May 18, 2021

fjmolinas commented Jun 11, 2021

Citrullin commented Aug 25, 2021

silkeh commented Sep 1, 2021

Citrullin commented Sep 22, 2021 •

edited

Loading

silkeh commented Sep 22, 2021 •

edited

Loading

kaspar030 commented Feb 1, 2022

fjmolinas commented Feb 1, 2022

fjmolinas commented Feb 1, 2022

fjmolinas commented Feb 1, 2022

fjmolinas commented Feb 1, 2022

silkeh commented Feb 9, 2022 •

edited

Loading

fjmolinas commented Feb 9, 2022

silkeh commented Feb 9, 2022

fjmolinas commented Feb 9, 2022

silkeh commented Feb 9, 2022

fjmolinas commented Feb 11, 2022

fjmolinas commented Feb 11, 2022

fjmolinas commented Feb 11, 2022

fjmolinas commented Feb 11, 2022

fjmolinas commented Feb 11, 2022

chrysn commented Feb 11, 2022

fjmolinas left a comment

fjmolinas commented Feb 11, 2022

fjmolinas commented Feb 11, 2022

silkeh commented Feb 12, 2022

	inline senml_numeric_t senml_float(float v)
	static inline senml_numeric_t senml_float(float v)

sys/senml: add SenML modules #16384

sys/senml: add SenML modules #16384

Conversation

silkeh commented Apr 24, 2021 • edited Loading

Contribution description

Testing procedure

Issues/PRs references

fjmolinas left a comment

Choose a reason for hiding this comment

fjmolinas May 7, 2021

Choose a reason for hiding this comment

fjmolinas May 18, 2021

Choose a reason for hiding this comment

fjmolinas May 18, 2021

Choose a reason for hiding this comment

fjmolinas May 18, 2021

Choose a reason for hiding this comment

fjmolinas commented Jun 11, 2021

Citrullin commented Aug 25, 2021

silkeh commented Sep 1, 2021

Citrullin commented Sep 22, 2021 • edited Loading

silkeh commented Sep 22, 2021 • edited Loading

kaspar030 commented Feb 1, 2022

fjmolinas commented Feb 1, 2022

fjmolinas commented Feb 1, 2022

fjmolinas commented Feb 1, 2022

fjmolinas commented Feb 1, 2022

silkeh commented Feb 9, 2022 • edited Loading

fjmolinas commented Feb 9, 2022

silkeh commented Feb 9, 2022

fjmolinas commented Feb 9, 2022

silkeh commented Feb 9, 2022

fjmolinas commented Feb 11, 2022

fjmolinas commented Feb 11, 2022

fjmolinas commented Feb 11, 2022

fjmolinas commented Feb 11, 2022

fjmolinas commented Feb 11, 2022

chrysn commented Feb 11, 2022

fjmolinas left a comment

Choose a reason for hiding this comment

fjmolinas commented Feb 11, 2022

fjmolinas commented Feb 11, 2022

silkeh commented Feb 12, 2022

silkeh commented Apr 24, 2021 •

edited

Loading

Citrullin commented Sep 22, 2021 •

edited

Loading

silkeh commented Sep 22, 2021 •

edited

Loading

silkeh commented Feb 9, 2022 •

edited

Loading