-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
App settings again #90
Conversation
@richibrics Take a look, I think it's working now. I copied the code for the default Logs folder paths from #61 I will update the top posts with listing what actually changed. E.g. you can edit settings in the UI now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is some code in Configurator
where it calculates the the height of the terminal, it could be moved here. Also the width of the terminal could be calculated here for Logging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it could be moved there since it's a terminal related class and we would free the Configurator of extra things
Hi @infeeeee , sorry for the late reply. |
Keep them away for now. I'm not sure they are needed at all, simply using |
Rewrote ClassManager. Settings work similarly to Warehouses and Entities. Settings are loaded to the SettingsManager singleton on startup, entities can ask configuration from there. Log settings loaded separately to Logger. @richibrics Take look, I consider this ready to be pulled now. |
|
||
|
||
class AppSettings(ConfiguratorObject): | ||
"""Singleton for storing AppSettings, not related to Entites or Warehouses """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not singleton, should update docstring
@richibrics This PR grew huge. Should I separate it to smaller ones, so it's easier to review? I can create 2 or maybe 3 smaller PRs instead of this beast. What do you think? |
Well if it's splittable it would be better for the understanding, but is it doable ? |
Yes, I think I can separate to these parts:
I won't close this so I can track changes in the other PRs. |
This PR monster will be separated to smaller separate requests:
Changes:
Configurator Menu
: Log Settings and App Settings under a common menu item Settings.TerminalDetection
class for terminal related methods.Changes for existing users:
Secondary Update interval is commented out yet. I figured out that you cannot change the update interval on sensor level, only on entity level, and I think it would make more sense if it could be changed on a sensor level. That way an entity could have one frequent sensor and one infrequent. What do you think?
Retry #84
TODO:
- [x] Logging starts with a default log level before loading the configuration- [x] After config loaded log level changes to the one in setting- [x] Add setting to enable/disable file logging- [x] Add setting for path to logdir- [x] Logging should be saved to some cache at the beginning. If file logging is enabled, save the cache to file, so nothing is lost. If file logging is not enabled drop the cache.- [ ] Implement secondary update interval in some sensors.