metaMMA commented 4 years ago

Changelog

Added configuration webpage functionality
- html file
- css styling
- jquery import and custom json_serialize .js file
- python script using cherrypy
Upgraded config.py to config.json
Added upgrade script and script directory
Amended requirements.txt, setup.pp
Added PiWeatherRockConfig service file to run the config page python script
Removed unused weather.conf file
Updated version number
Fixed typo in weather.py
Amended travis.yml to tell pylint not to look at config.py
Amended .gitignore to ignore config.json
Added lightning background for config web page
Replace references to config.py variables with config.json variables in weather.py

metaMMA commented 4 years ago

I'm giving up on pleasing Travis, for now.

metaMMA commented 4 years ago

So... I have not dug into see where all it is used, but I think I saw that you are planning to both include a .lock file and write to it yet I don't see it in the .gitignore. This would cause a dirty working directory and should be avoided. You may want to place it in /var/run/, /var/run/PiWeatherRock/ or add it to the ignore file. Really open on where to put it so long as it doesn't make for a dirty git directory

Yeah, the .log and .lock file are not in this PR, but will be added to .gitignore when they show up in a PR. I'm leaning toward putting them in the root dir, because we know there won't be any permission issues (especially if later version support macOS or other GNU/Linux distros)

genebean commented 4 years ago

Looks like you have addressed all the bits I asked you too... are you ready for me to start testing this locally and then merge?

metaMMA commented 4 years ago

Looks like you have addressed all the bits I asked you too... are you ready for me to start testing this locally and then merge?

Yeah, it seems ready for testing. We might be able to recruit some people to test from gitter.

genebean commented 4 years ago

I will give it a go tomorrow then.

genebean commented 4 years ago

Also, all the numbers in the config file are currently saved as strings which makes them not work as expected. Unquoting them fixed it. Upon last check even if I pick true on the config page it doesn't change "fullscreen": "True" to "fullscreen": true like it should. I imagine if the initial value had not been wrong it would have though.

metaMMA commented 4 years ago

Also, all the numbers in the config file are currently saved as strings which makes them not work as expected. Unquoting them fixed it. Upon last check even if I pick true on the config page it doesn't change "fullscreen": "True" to "fullscreen": true like it should. I imagine if the initial value had not been wrong it would have though.

Yes, I see now that my script for importing variables from config.py imported everything as a string. Everything should be fixed now. I'm hoping the fullscreen issue was related to it not liking the string when it was expecting a boolean. It seems to be working for me, now.

genebean commented 4 years ago

Sweet. I will test shortly.

metaMMA commented 4 years ago

So close...

I made the following edit and then ran puppet to apply the changes.
diff --git a/setup.pp b/setup.pp
index b936855..3bbffe9 100644
--- a/setup.pp
+++ b/setup.pp
@@ -86,9 +86,10 @@ exec { 'enable display-setup-script':
 }

 vcsrepo { '/home/pi/PiWeatherRock':
-  ensure   => latest,
+  ensure   => present,
   provider => git,
   source   => 'https://github.com/genebean/PiWeatherRock.git',
+  revision => 'f45c307ace738018fa592074bbf5f5d6d2239dce',
 }
After that, the config file looked good and both services work. The only issue I currently see is that updates made via the web interface don't persist. It says it succeed but neither toggling fullscreen or adjusting the frequency actually saved into the config.

In some future PRs I have a way of re-importing an updated config file while the service is running. As of this PR, weather.py loads the config when it starts and does not have a method to reload without restarting the service. I'm hoping that is the issue.

It might be advantageous to add the start/stop to the config page so that the config can re-import while the service is running. It's not a trivial change, but not super complicated either. Just need to add a way to check .lock file and wrap most of the code in a while loop that either waits for signal from .lock file to start running (thus re-importing the config).

genebean commented 4 years ago

@metaMMA I was looking at the conf file on disk and there were no edits making it there.

metaMMA commented 4 years ago

Nothing left but to squash. Do you want to do it or shall I. I honestly have no preference.

One last thing. I didn't change anything here. Not sure if it needs to be changed, though. If it's good, then I say go for it!

diff --git a/setup.pp b/setup.pp
index b936855..3bbffe9 100644
--- a/setup.pp
+++ b/setup.pp
@@ -86,9 +86,10 @@ exec { 'enable display-setup-script':
 }

 vcsrepo { '/home/pi/PiWeatherRock':
-  ensure   => latest,
+  ensure   => present,
   provider => git,
   source   => 'https://github.com/genebean/PiWeatherRock.git',
+  revision => 'f45c307ace738018fa592074bbf5f5d6d2239dce',
 }

genebean commented 4 years ago

That was just for testing. All good as is. I will get to squashing

genebean / PiWeatherRock

New config #46

Changelog