From 018a651d28471e4f2207743f6178f83c0b3ed7dc Mon Sep 17 00:00:00 2001 From: Emma Nora Theuer Date: Wed, 16 Oct 2024 19:39:13 +0200 Subject: [PATCH] Add various TODOs. --- src/wallman_lib.py | 47 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/src/wallman_lib.py b/src/wallman_lib.py index fbaea28..284d788 100644 --- a/src/wallman_lib.py +++ b/src/wallman_lib.py @@ -5,19 +5,14 @@ import tomllib from datetime import datetime, time from apscheduler.triggers.cron import CronTrigger -# setup logging +# Setup Logging. NOTE: Declaration as a global variable is necessary to ensure correct functionality across multiple modules. logger = logging.getLogger("wallman") class ConfigError(Exception): pass class _ConfigLib: - def _initialize_config(self) -> dict: - chdir(str(getenv("HOME")) + "/.config/wallman/") - with open("wallman.toml", "rb") as config_file: - data = tomllib.load(config_file) - return data - + # Initializes the most important config values. TODO: Add handling for the empty config case def __init__(self): self.config_file: dict = self._initialize_config() # Full config # Dictionaries @@ -29,10 +24,11 @@ class _ConfigLib: self.config_wallpapers_per_set: int = self.config_general["wallpapers_per_set"] self.config_total_changing_times: int = len(self.config_changing_times) self.config_log_level: str = self.config_general.get("loglevel", "INFO").upper() + # HACK: Add a function to handle these try/except blocks cleanlier. try: self.config_notify: bool = self.config_general["notify"] except KeyError: - self.config_notify = False + self.config_notify: bool = False logger.warning("'notify' is not set in dictionary general in the config file, defaulting to 'false'.") try: self.config_systray = self.config_general["systray"] @@ -40,12 +36,20 @@ class _ConfigLib: self.config_systray = True logger.warning("'systray' is not set in the dictionary general in the config file, defaulting to 'true'.") - # Setup logging self._set_log_level() + # Setup systray. if self.config_systray: self._initialize_systray() + # Read config. TODO: Add error handling for the config not found case. + def _initialize_config(self) -> dict: + chdir(str(getenv("HOME")) + "/.config/wallman/") + with open("wallman.toml", "rb") as config_file: + data = tomllib.load(config_file) + return data + + # HACK on this to avoid double importing of wallman_systray due to variable scope. Idea: Global variable or Variable that is inherited? def _initialize_systray(self): try: import wallman_systray @@ -69,6 +73,7 @@ class _ConfigLib: raise ConfigError("An error occured and no fallback wallpaper has been set, exiting...") class ConfigValidity(_ConfigLib): + # TODO: Add handling for the empty config case. def __init__(self): super().__init__() @@ -96,6 +101,8 @@ class ConfigValidity(_ConfigLib): raise ConfigError("Please provide an amount of changing_times equal to wallpapers_per_set, exiting...") def _check_general_validity(self) -> bool: + # FIXME! + # HACK: Adjust it to check for the actually required variables existing rather than check if a number of options is set, which is highly error prone. if len(self.config_general) < 3: try: self._set_fallback_wallpaper() @@ -116,6 +123,7 @@ class ConfigValidity(_ConfigLib): if wallpaper_set in self.config_file: logger.debug(f"The dictionary {wallpaper_set} has been found in config.") return True + # TODO split this into smaller pieces. This goes too deep. else: try: self._set_fallback_wallpaper() @@ -143,6 +151,8 @@ class ConfigValidity(_ConfigLib): raise ConfigError(f"Dictionary {wallpaper_set} does not have the correct amount of entries, exciting...") def validate_config(self) -> bool: + # NOTE: Consider changing this to exit(-1) + # HACK: Consider using different exit codes for different errors to help users debug. if not self._check_fallback_wallpaper(): pass if not self._check_wallpapers_per_set_and_changing_times(): @@ -156,22 +166,28 @@ class ConfigValidity(_ConfigLib): logger.debug("The config file has been validated successfully (No Errors)") return True +# TODO: Improve modularity. See notes inside the class for more details. +# TODO: Ensure functionality and if needed add handling for the 1 wallpaper per set case. class WallpaperLogic(_ConfigLib): def __init__(self): super().__init__() + # NOTE: This looks a bit ugly. Consider pros and cons of adding this into _ConfigLib self.chosen_wallpaper_set = False + # NOTE: This function could be in a different file because it's not needed in the case only 1 wallpaper per set is needed. # Returns a list of a split string that contains a changing time from the config file def _clean_times(self, desired_time) -> list: unclean_times = list(self.config_changing_times.values())[desired_time] return unclean_times.split(":") + # NOTE: This could be in a different file because it's not needed in the "Only one wallpaper set" case. def _choose_wallpaper_set(self) -> None: from random import choice as choose_from self.chosen_wallpaper_set = choose_from(self.config_used_sets) self.wallpaper_list = list(self.config_file[self.chosen_wallpaper_set].values()) logger.debug(f"Chose wallpaper set {self.chosen_wallpaper_set}") + # NOTE: Same as _clean_times() # Verify if a given time is in a given range def _time_in_range(self, start, end, x) -> bool: if start <= end: @@ -179,6 +195,7 @@ class WallpaperLogic(_ConfigLib): else: return start <= x or x < end + # NOTE: Potentially add handling for this to be also usable for notify_user and add logging if notify_user fails. Consider adding an argument that is where it's called from and handle accordingly. def _check_system_exitcode(self, code) -> bool: if code != 0: try: @@ -194,10 +211,15 @@ class WallpaperLogic(_ConfigLib): logger.info(f"The wallpaper {self.wallpaper_list[self.current_time_range]} has been set.") return True + # NOTE: Add error handling in case libnotify is not installed or notify-send fails for any other reason. + # TODO: Add a check whether config[notify] is true or not. def _notify_user(self): system("notify-send 'Wallman' 'A new Wallpaper has been set.'") logger.debug("Sent desktop notification.") + # TODO: Clean this up. It's way too large and way too intimidating. + # NOTE: This could be in a different for the case that the user only wants 1 wallpaper per set. + # TODO: Add an way for the user to choose if the wallpaper should scale, fill or otherwise. This needs to be editable in the config file. def set_wallpaper_by_time(self) -> bool: # Ensure use of a consistent wallpaper set if self.chosen_wallpaper_set is False: @@ -206,11 +228,14 @@ class WallpaperLogic(_ConfigLib): self.current_time_range = time_range # Store current time for better debugging output clean_time = self._clean_times(time_range) clean_time_two = self._clean_times(time_range + 1) + # HACK on this to make it more readable. This function call is way too long. Consider storing these in a bunch of temporary variables, though keep function length in mind. + # HACK on this to see if this logic can be simplified. It's very ugly to check it that way. # Check if the current time is between a given and the following changing time and if so, set that wallpaper. If not, keep trying. if self._time_in_range(time(int(clean_time[0]), int(clean_time[1]), int(clean_time[2])), time(int(clean_time_two[0]), int(clean_time_two[1]), int(clean_time_two[2])), datetime.now().time()): system(f"feh --bg-scale --no-fehbg --quiet {self.wallpaper_list[time_range]}") exitcode = system(f"feh --bg-scale --no-fehbg --quiet {self.wallpaper_list[time_range]}") has_wallpaper_been_set = self._check_system_exitcode(exitcode) + # TODO: Add this check to _notify_user. if self.config_notify: self._notify_user() return has_wallpaper_been_set @@ -224,11 +249,13 @@ class WallpaperLogic(_ConfigLib): self._notify_user() return has_wallpaper_been_set + # NOTE: Consider avoiding nested functions. def schedule_wallpapers(self): def _schedule_background_wallpapers(): from apscheduler.schedulers.background import BackgroundScheduler scheduler = BackgroundScheduler() # Create a scheduled job for every changing time + # NOTE: This should be a function. for changing_time in range(len(self.config_changing_times)): clean_time = self._clean_times(changing_time) scheduler.add_job(self.set_wallpaper_by_time, trigger=CronTrigger(hour=clean_time[0], minute=clean_time[1], second=clean_time[2])) @@ -240,6 +267,7 @@ class WallpaperLogic(_ConfigLib): from apscheduler.schedulers.blocking import BlockingScheduler scheduler = BlockingScheduler() # Create a scheduled job for every changing time + # NOTE: Thisshould be a function. for changing_time in range(len(self.config_changing_times)): clean_time = self._clean_times(changing_time) scheduler.add_job(self.set_wallpaper_by_time, trigger=CronTrigger(hour=clean_time[0], minute=clean_time[1], second=clean_time[2])) @@ -247,6 +275,7 @@ class WallpaperLogic(_ConfigLib): scheduler.start() if self.config_systray: + # NOTE: The wallman_systray impomrt should be handled differently. See the note in Config_Validity. import wallman_systray as systray from functools import partial scheduler = _schedule_background_wallpapers()