thomaspasser / gpio-buttons

Control volumio2 on RPi with GPIO buttons
http://tomatpasser.dk/gpio-buttons.zip
52 stars 19 forks source link

Default configuration file is not used #14

Closed thomaspasser closed 7 years ago

thomaspasser commented 7 years ago

At this moment the default values in config.json is not used, but a iterative loop defines the default values. This makes it impossible to assign default values to functions where it makes sense.

Shutdown: GPIO3

macmpi commented 7 years ago

Looked a bit at the possible issue. It seems gpio-buttons does not read/access/write .json files with normal accessor functions (like config.get(item) or config.set(item, value)). Instead gpio-buttons uses a flattened representation. For exemple, instead of config.json looking as:

{
  "playpause":{
    "enabled": {"value": false, "type": "boolean"},
    "pin": {"value":"17","type":"number"},
    "value": {"value": "0", "type": "number"}
  },
  "volup":{
    "enabled": {"value": false, "type": "boolean"},
    "pin": {"value":"18","type":"number"},
    "value": {"value": "0", "type": "number"}
  },
  "voldown":{
    "enabled": {"value": false, "type": "boolean"},
    "pin": {"value":"22","type":"number"},
    "value": {"value": "0", "type": "number"}
  },
  "previous":{
    "enabled": {"value": false, "type": "boolean"},
    "pin": {"value":"23","type":"number"},
    "value": {"value": "0", "type": "number"}
  },
  "next":{
    "enabled": {"value": false, "type": "boolean"},
    "pin": {"value":"24","type":"number"},
    "value": {"value": "0", "type": "number"}
  },
  "shutdown":{
    "enabled": {"value": false, "type": "boolean"},
    "pin": {"value":"3","type":"number"},
    "value": {"value": "0", "type": "number"}
  }
}

it looks as: "{\"playpause\":{\"enabled\":true,\"pin\":17,\"value\":0},\"volup\":{\"enabled\":false,\"pin\":18,\"value\":0},\"voldown\":{\"enabled\":false,\"pin\":22,\"value\":0},\"previous\":{\"enabled\":false,\"pin\":23,\"value\":0},\"next\":{\"enabled\":false,\"pin\":24,\"value\":0},\"shutdown\":{\"enabled\":false,\"pin\":3,\"value\":0}}" because it uses "flattened stringified" representation in read/write file functions.

Therefore, at first read, default standard .json is probably not decoded properly, and getconf then generates other defaults. After this is saved once, those flattened .json files are read/written consistently (but do not look the traditional way).

So you may want to use more traditional .json accessors, like other plugins do, so that .json file format always stays in classical form.

thomaspasser commented 7 years ago

I fixed this issue in the 'config-rewrite' branch, will merge with master soon!

macmpi commented 7 years ago

Looks good, thanks (did not test formally yet). Actually I think pluginmanager would even create /data/configuration/miscellanea/gpio-buttons directory itself, so it may not be necessary to create it in install.sh: you may test a fresh install removing the directory altogether.

macmpi commented 7 years ago

Tested 'config-rewrite' branch, and seems good: now initial prefs are good. Don't you want to use defaults as per github code page (17 play, 18 vol+, 22 vol-, 23 prev, 24 next, 3 shutdown), so that users get some consistency with "doc"?

Only annoying stuff is that plugins remains reported as inactive (red light) in UI view: must be something related to premises handling in onStart or something. Don't think it's related to this change (maybe to some of my earlier?)

thomaspasser commented 7 years ago

We could do that, yes.

About the plugin reporting as inactive: I'm not sure about what is causing it, maybe because we changed

return libQ.resolve();

to

return defer.promise;

in onVolumioStart. But all the other plugins use return defer.promise;, thats why we changed it..

macmpi commented 7 years ago

Yes, I'm not familiar with node either: tried to look a some other plugins, and I suspect this green button is not a UI cosmetic-only issue, and may have consequences on overall plugins operations. So better getting this right.

Actually maybe 503_hta_tube_amp from @volumio is simple enough and can serve as a good example.

macmpi commented 7 years ago

thanks!