[riot-notifications] [RIOT-OS/RIOT] sys/senml: add SenML modules (#16384)

Silke Hofstra notifications at github.com
Wed Sep 1 13:41:20 CEST 2021


> 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:
> 
> ```c
> 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:
> ```c
> 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:

```c
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:
```c
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);
```




-- 
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/16384#issuecomment-910202747
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20210901/e2d9487a/attachment.htm>


More information about the notifications mailing list