nerves-project / nerves_runtime

Small, general initialization and utilities for Nerves devices
Apache License 2.0
57 stars 23 forks source link

Inconsistent return value for `Nerves.Runtime.KV.get/1` on RPi4 after reboot #189

Closed psteininger closed 2 years ago

psteininger commented 4 years ago

Environment

* Additional information about your host, target hardware or environment that
  may help

### Current behavior
When I set a custom firmware Key/Value pair as a list, it is returned as a list until a reboot. 

Nerves.Runtime.KV.put "test", [17,22,27]

After the reboot, what is returned is a `BitString`

iex(3)> Nerves.Runtime.KV.get "test"
<<17, 22, 27>>


### Expected behavior
Either prevent me from setting the value of the key as a list or keep returning it as a list.
mobileoverlord commented 4 years ago

I am unsure how to best handle this. lists of integers is a valid bit string syntax. I think this should get handled in documentation instead of limiting the API.

fhunleth commented 4 years ago

We currently spec the put* functions as taking String.t(). (put/1's spec could be improved).

I think that it might be ok to raise if strings aren't passed. @mobileoverlord - per your comment about lists of integers, I assume that you're referring to iolists/iodata. I'm not 100% sure that iodata makes it though. In this case, it seems a little confusing to support iodata, though. I'll ponder more...

fhunleth commented 4 years ago

Ok, I took a look at the KV specs, and a few of them are incorrect. I'd like to fix this first, so that any guards we add are accurate.

@psteininger Thanks for bringing this to our attention!

psteininger commented 4 years ago

Thanks for looking into it @fhunleth. When I first set a list in the KV I was really excited I would love to be able to set not only lists, but hashes and tuples or lists with tuples.

How difficult would it be to add support for that?

My use case is to "encode" my image with some configuration - the project I am working on is a soil moisture monitoring and plant watering. I wanted to set up each station with the specific GPIO pins to be used for controlling the relays. I also wanted to configure which I2C addresses and analog inputs on the ADS1115 are active and which GPIO input pin is tied to which ADS1115 alert pin.

I could provision the devices from a hub app they will report to, but I would rather that kind of internals be baked in on the image.

fhunleth commented 4 years ago

The U-Boot environment block is used to interop with the U-Boot bootloader on other devices. I think that some thought would be needed to make this generic without breaking variables shared with the bootloader.

I'd recommend handling the encoding and decoding of these yourself. The environment block format does have some constraints: Keys can't contain = signs, nothing can contain zero bytes (it originated in C, so everything's NULL terminated), and if you were on a U-Boot device (like the BBB or STM32MP1), I'd probably stay with ASCII.

fhunleth commented 2 years ago

Closing old issues. Please comment if you'd like to revisit.