Add various TODOs.

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:
+    # Initializes the most important config values. TODO: Add handling for the empty config case
-        chdir(str(getenv("HOME")) + "/.config/wallman/")
-        with open("wallman.toml", "rb") as config_file:
-            data = tomllib.load(config_file)
-            return data
-
     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()