naofireblade / homebridge-weather-plus

A comprehensive weather plugin for homebridge.
MIT License
311 stars 61 forks source link

Wondered why my threshold entries were empty in the settings #294

Open Tomcraft1980 opened 1 month ago

Tomcraft1980 commented 1 month ago

You changed the names for the threshold config, see: https://github.com/naofireblade/homebridge-weather-plus/compare/v3.3.3...master#diff-be1695b1e63a508d59982601f9e1fb7f58247deecb1e427adb77bcad758ae5e5 It was for example "tresholdAirPressure" in plugin version 3.3.2 Since 3.3.3 you corrected the typo and named it "thresholdAirPressure". This results in empty settings for the threshold sensors. No big deal but would have been nice to be noticed about this. ;-)

dacarson commented 1 month ago

Yes. The configuration page was unfortunately broken. Because there was one spelling for the field where you entered the value, and then another name in the config file. So the value in the config file isn't loaded into the configuration page and vice versa, the value entered in the configuration page wasn't save. So unless you manually modified the JSON config file, it wasn't working. I should have looked into how to migrate bad settings. Sorry about that.

Tomcraft1980 commented 1 month ago

No problem, I think this can still be done to migrate the settings, as others will not have noticed this problem yet due to issue #247. ;-) Thanks for looking into it.

dacarson commented 1 month ago

I started to investigate a migration path, and I wonder if this would work. I would add the correct spelling rather than correcting the spelling. The result is that the old fields and new fields will appear in the config.json file.

So I would put this back:

      "tresholdAirPressure": {
       "type": "integer"
     },
     "tresholdCloudCover": {
       "type": "integer"
     },
     "tresholdUvIndex": {
       "type": "integer"
     },
     "tresholdWindSpeed": {
       "type": "integer"
     }

and add this: "thresholdAirPressure": { "type": "integer" }, "thresholdCloudCover": { "type": "integer" }, "thresholdUvIndex": { "type": "integer" }, "thresholdWindSpeed": { "type": "integer" }

Update the usage code to prefer the correct spelling values (ie the ones entered through the UI), ie:

  station.thresholdAirPressure = stationConfig.thresholdAirPressure || station.tresholdAirPressure
  station.thresholdCloudCover = stationConfig.thresholdCloudCover || station.tresholdCloudCover
  station.thresholdUvIndex = stationConfig.thresholdUvIndex || station.tresholdUvIndex
  station.thresholdWindSpeed = stationConfig.thresholdWindSpeed || station.tresholdWindSpeed

The old data will not appear Configuration UI, but if you do enter a threshold values in the Configuration UI then it will use it. It'll just leave behind the old data - which could be confusing.

Tomcraft1980 commented 1 month ago

I think this would be confusing if someone has the old settings, which then would be read correctly, but not be displayed in the settings. After re-editing the setting via the UI he would end up with the old and the new spelling in the config.json, right? Is there a way to manipulate the config.json? Otherweise just put a note in the next update to be aware of the changes of the spelling since v3.3.3

dacarson commented 1 month ago

Agree that it would be confusing. I have not yet seen a way to manipulate the config.json.

Tomcraft1980 commented 1 month ago

Then just drop a small note in the update section, so that users are aware this. ;-)