AW: [PATCH] Documentation: state: add a little note about modifying a state variable in the frontend

[Matthias Fend]

Hi Sascha,

> ________________________________________
> Von: Sascha Hauer <s.hauer at pengutronix.de>
> Gesendet: Montag, 9. November 2020 10:11
> An: Matthias Fend
> Cc: Ahmad Fatoum; barebox at lists.infradead.org
> Betreff: Re: [PATCH] Documentation: state: add a little note about modifying a state variable in the frontend
> 
> On Fri, Nov 06, 2020 at 07:14:09AM +0000, Matthias Fend wrote:
>> Hi Ahmad,
>>
>>> ________________________________________
>>> Von: Ahmad Fatoum <a.fatoum at pengutronix.de>
>>> Gesendet: Donnerstag, 5. November 2020 21:57
>>> An: Matthias Fend; barebox at lists.infradead.org
>>> Betreff: Re: [PATCH] Documentation: state: add a little note about modifying a state variable in the frontend
>>>
>>> Hello Matthias,
>>>
>>> On 11/5/20 9:16 PM, Matthias Fend wrote:
>>>> Signed-off-by: Matthias Fend <matthias.fend at wolfvision.net>
>>>> ---
>>>>    Documentation/user/state.rst | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/Documentation/user/state.rst b/Documentation/user/state.rst
>>>> index 78ce24f..d9a8e76 100644
>>>> --- a/Documentation/user/state.rst
>>>> +++ b/Documentation/user/state.rst
>>>> @@ -689,4 +689,5 @@ be accessed like normal shell variables. The ``state`` command is used
>>>>    to save/restore a *state* variable set to the backend device.
>>>>
>>>>    After initializing the variable can be accessed with ``$state.foo``.
>>>> +To modify the value of a state variable use ``nv dev.state.foo=bar``.
>>>
>>> That's wrong. You set the value of a state variable with
>>> state.foo=bar. nv dev.state.state.foo=bar uses the _environment_ to
>>> save a value to write to state.foo on each boot. This means:
>>
>> Thanks for your explanation. Setting a state variable with
>> 'state.foo=bar' was actually the way I originally expected.
>> Meanwhile, I figured out why I was struggling using things like that.
>> All my introduced state variables contained a hyphen in their name.
>> And I was not able to set a state variable with 'state.foo-bar=baz' while
>> 'nv dev.state.foo-bar=baz' immediately changed the value of
>> 'state.foo-bar'. But, as I just learned, with some side effects.
>>
>>>
>>>    - If you modify the variable from within barebox, it won't be visible
>>>      in Linux unless you boot barebox once again.
>>
>> I could not verify this behavior. When I change a state variable with
>> 'nv dev.state=foo' and then directly boot into Linux (without booting
>> barebox again) the 'barebox-state' tool already displays the new value.
>>
>>>    - If you modify the variable from within Linux, nv.dev.state.state.foo
>>>      would just overwrite the existing value on the next barebox boot.
>>
>> That's true, and of course, not desired.
>>
>>>
>>> Generally, you should only need to modify nv variables at runtime
>>> for debugging. nv.dev is meant for stuff like nv.dev.eth0.ethaddr=XX:XX:XX:XX:XX:XX,
>>> where you can just hard code a mac address to use, but for state I can't
>>> think of any good use cases at the moment.
>>
>> Right, state, and also it's frontend, does already what it should do and
>> as expected.
>> Only the hyphen-obstacle caused troubles. So, maybe there is a trick but the fast fix is to simply
>> just avoid names that contain hyphens ;)
> 
> How about converting the hyphens to underscores for the variable names
> (I mean in code, not in your dts files)?

Things are clearer to me now. Since most shells do not allow variable names that contain such characters, one could also guess that the shell integration does not work for such state variables.

Changing the variable name automatically would work, but since this is just a workaround, I don't know if this is a good idea.
And this might also break some existing stuff.

Since the names of the state variables can be chosen freely, I still think it is better to just avoid "bad" characters. After adding new status variables, these will most likely be tested by playing around in the Barebox console. This is where you will find out at the latest that you did something wrong.

Maybe an improvement would be to leave a note in the chapter "Variable subnodes".

Thanks,
 ~Matthias

> 
> Sascha
> 
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | https://eur04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pengutronix.de%2F&data=04%7C01%7Cmatthias.fend%40wolfvision.net%7C0f554bd59a134feed96d08d8848f7f74%7Ce94ec9da9183471e83b351baa8eb804f%7C0%7C0%7C637405099148599343%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=w%2FNxxu9TUXOPR3m8FWqSNxJBrgUqvTj%2F6IW6OmFWNQc%3D&reserved=0  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>