Skip to content
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

Feature: dynamic modules (no location preference, no preferred position) #94

Closed
Atrejoe opened this issue Apr 12, 2020 · 20 comments
Closed
Assignees
Labels
Beta branch (dev) Related to the dev branch (beta). Mainly for developers... Done! Something has been achieved!

Comments

@Atrejoe
Copy link
Collaborator

Atrejoe commented Apr 12, 2020

Panel order should dynamic: panels (modules) should be told their dimensions instead of determining themselves based on figuring out their position by reading from settings.py.

Any logic that is based on the position (where a panel should know if is it at the top, middle or bottom) is yet to be determined, if to exist at all.

This would also allow changing the number of panels (with a minimum of one of course)

@aceisace
Copy link
Collaborator

@Atrejoe I've tried implementing the requested features without any major changes on the software file, but at this point, it's more easier to refactor the code than modifying each file.
I'll try to do some refactoring to see how well the new features and suggestions can be implemented.

@aceisace aceisace self-assigned this Apr 16, 2020
@aceisace aceisace added the in progress Something is currently being tested. label Apr 16, 2020
@aceisace
Copy link
Collaborator

aceisace commented Apr 29, 2020

@Atrejoe
OK, the first part of the refactoring is done. So far, this has been achieved on my local build:

  • Restructured folder and files to allow using Inky-Calendar as a python package (-> import inkycal)
  • Set up Inky-Calendar as a package in development mode (pip3 install -e ....) (equivalent to virtual env)
  • Use JSON settings file (can be located anywhere, path now has to be specified)
  • Validate JSON settings file
  • Dumb modules, no more fixed display location or fixed sizes
  • Work in progress: Runnning without display
  • Work in progress: inkycal_...modules refactoring (including inkycal_image)

I'll implement the features from the dev branch locally as well. Once I make some more progress, I'll post an update.

@Atrejoe
Copy link
Collaborator Author

Atrejoe commented Apr 30, 2020

That is not a step, that is a leap! Looking forward to give it a spin!

@aceisace
Copy link
Collaborator

@Atrejoe Thanks! It's still going to take a bit longer before it will work as expected (not too familiar with classes, packages etc.) but I'll try my best :D.

@Atrejoe
Copy link
Collaborator Author

Atrejoe commented May 10, 2020

I have reviewed some code and have some suggestions:

  1. I like that you have started compartmentalizing the different parts of logic. It's really nice that we can test the settings-parser stand-alone. This is a first step towards unit-tests.
  2. I suggest abandoning the named (location top, middle, bottom) section approach. Instead of defining three sets of variables for a fixed set three sections, use a list or array of sections. When just specifying a number of ordered sections, this allows you to have a one, two, three, four or n-section panels with ease. Initially we should start with a linear number of sections:
+---+     +---+
| 1 |     |   |
+---+     |   |
| 2 |     |   |
+---+  or | 1 |
| 3 |     |   |
+---+     |   |
| 4 |     |   |
+---+     +---+

, but later we could migrate to a grid:

+---+---+
| 1 | 2 |
+-------+
|   3   |
+-------+
|   | 5 |
| 4 |---+
|   | 6 |
+---+---+
  1. Currently the lookup for modules it by their name, this results in allowing a panels to appear only on one positions. When following (1) this would also allow a section of the same type to be displayed multiple time with different configs. Maybe the approach would have to be "find the module for the config" instead of "find the config for this module".
  2. You already have introduced configurable relative height I think, we could easily make this optionally configurable in settings.json. I suggest allowing an optional default relative number to be set. When not set, it should assume a relatively high number, like 100. This will allow people to have pixel-perfect control, when they want to, but could also use smaller numbers for defining relative section dimensions.
    1. Example I - no heights set
      Heights are not set for any of three configured sections, the panel is 800 px high. The panels then would automatically be set to 266, 266 and 267 px.
    2. Example III - absolute heights set for all sections
      Heights are set for three configured sections as 200 : 250 : 350, the panel is 800 px high. The panels then would match their heights in pixels..
    3. Example III - relative heights set for all sections
      Heights are set for three configured sections as 1 : 2 : 2, the panel is 800 px high. The panels then would automatically be set to 160, 320 and 320 px.
    4. Example IV - height set for all one section (unusual)
      Height is set for the first of three configured sections as 500, the panel is 800 px high. The panels then would automatically be set to 500 : 100 : 100, resulting in 571,114 and 115 px.
  3. Module requirements: The RSS module already looks fine and independent of the location. However when specifying what interface or class any module should implement, dynamic executions of module should be possible. Inkycal.py should not know about concrete implementations like the RSS component, image component or weather. There should be a lookup from the configured panel.type to a module that goes by the name. The module (class) could then have implemented thing like:
    • a constructor that takes basic argument like height, width and color palette.
    • a "configure" method that accepts section-specific config, which also validates that config. (similar to settings_parser.py)
    • a "render image" method
....
from abc import ABC, abstractmethod

class section(ABC):
  # A base class for all Inkycal section implementations

  def __init__(self, section_size, section_config):
    # Initializes base section,
    # sets properties shared amongst all sections
    self.name = os.path.basename(__file__).split('.py')[0]
    self.width, self.height = section_size

    # Validate and store section-specific config
    self.configure(section_config)

  @abstractmethod
  def configure(self,section_config):
    # Implementing method should validate and store config
    raise NotImplementedError
  
  @abstractmethod
  def generate_image(self):
    # Implementing method should generate one or more
    # images based on dimensions and configuration specified before.
    raise NotImplementedError

class inkycal_rss(section):
  # RSS section, showing an RSS feed

  def __init__(self, section_size, section_config):
    """Initialize inkycal_rss module"""
    super().__init__(self, section_size, section_config)

    # Set section-specific defaults
    self.background_colour =  'white'
    self.font_colour = 'black'
    self.fontsize = 12
    self.font = ImageFont.truetype(
      fonts['NotoSans-SemiCondensed'], size = self.fontsize)
    self.padding_x = 0.02
    self.padding_y = 0.05
    print('{0} loaded'.format(self.name))

  def configure(self,section_config):
    self.config = section_config # todo: validation, store individual variables

  def generate_image(self):
    """Generate image for this module"""

    # Define new image size with respect to padding
    im_width = int(self.width - (self.width * 2 * self.padding_x))
    im_height = int(self.height - (self.height * 2 * self.padding_y))
    im_size = im_width, im_height

    ....

This will probably mean that the dynamically determined module.name is on the late side.

Having said this, I think you are well underway towards this setup. The code looks much cleaner, testable and maintainable.

@Atrejoe
Copy link
Collaborator Author

Atrejoe commented May 10, 2020

For example purposes, I added a basic unit test:
0a46771

This does nothing really, but your IDE maybe will suggest running the test.

@aceisace
Copy link
Collaborator

@Atrejoe Thanks for the detailed feedback.

  • unnamed locations
    This sure would be nice to implement. Replacing section names with (section-)numbers seems easy to implement, but how will the web-ui handle an undefined number of sections?
    Do you have a plan on what steps to take to implement this feature?

  • module lookup by name
    this is the current json config for a module:

               {
			"location": "top",
			"type": "inkycal_weather",
			"config": {
				"api_key": "topsecret",
				"location": "Stuttgart, DE"
			}
		},

The new settings-parser parses all module-names (type) in the form of a list. The idea is that the main file (Inkycal.py) will check which modules to load by looking up this list, before passing on the config for that module for the initialisation. -> load module which requires this config.
Does this follow the approach you suggested?

  • section sizes
    Current function that defines default section sizes: layout-file
  def create_sections(self, top_section=0.10, middle_section=0.65,  bottom_section=0.25):
    """Allocate fixed percentage height for top and middle section
    e.g. 0.2 = 20% (Leave empty for default values)
    Set top/bottom_section to 0 to allocate more space for the middle section
    """

Allocates 10% to top-section, 65% to middle-section and 25% to bottom section (of total display height). Is this equivalent to setting a fixed relative (but changeable) height for each module?

If the height could be set optionally for each module on the web-ui (either by percentage or px), it would be nice. Maybe we could have this approach:

  1. check if height is specified in module config (in .json settings file)
  2. if it is, use this height, if not, use default height.
    Maybe you could help with adding this setting in the web-ui?
  • Module requirements
    I'm still new to advanced classes, but I like the idea of dynamic executions. Could you please provide an example on how this can be accomplished? Also, thanks for providing the interface (module template). I've saved it locally, so I'll try to get this working and then implement it in all the modules.

Last, but not least, glad to hear you like the new code :D. It's a big jump, but hopefully, this will make things easier later on :).

@aceisace
Copy link
Collaborator

@Atrejoe OK, had a go at this.
Tried inkycal_rss with the following init parameters:

rss = inkycal_rss((400,100), {'url':'dummy_url'})

But trying to init raises this Error:
TypeError: __init__() takes 3 positional arguments but 4 were given
Section size should be a tuple: (width, height), but this is not recognised correctly.

Am I doing something wrong or does the code need to be adjusted? Thanks in advance :)

@Atrejoe
Copy link
Collaborator Author

Atrejoe commented May 16, 2020

regarding:

....
But trying to init raises this Error:
TypeError: __init__() takes 3 positional arguments but 4 were given

Quite strange, again, I'm a python noob, but I'd say you are giving 2 arguments; a tuple and an object. What is you were to assign these to parameters first. (should not matter I think)
Can you share the stack trace and you code (or did you copy-paste mine exactly?

@Atrejoe
Copy link
Collaborator Author

Atrejoe commented May 16, 2020

  1. unnamed locations

I think, for now this is a shortcoming of the Web-UI: it will just configure a three-panel layer (or with minor tweaks maybe a one-two-or-three panel layout). We cán make this a client-side application, with dynamic panel configuration too, but for now this is too much effort for a small team, we have other stuff to fix.

An important reason for this is that I deem it too high of a threshold for non-dev users or day-to-day usage to have to upload a config file via ssh upon each change. This why I keep pushing for a server-side approach: the InkyCal web-service.
Up until now the features Inkycal web service provide are pretty much in sync with the client-side inky cal, I could have it spit out the same config file, which ís dynamic, but keeping server-and-client side features in sync would require some coordination.

Another possible route would be serving a configuration-website on the device itself and then generate the file on the device (you could then even implement features like user-triggered-reloading). Also this is not one of my favorites, but I can so why some people would like it.

Maybe we should have call one day on this.

  1. module lookup by name

I think your approach does match my description. This way you could reduce the number of loaded modules ánd use the same module multiple times with different config, right? See also (5)

  1. section sizes

Yes I can add this to the web UI, as this is very little effort. I'll add an optional numeric (integer) input.
In the current the Web-UI, which is three-panel only, I will pre-populate the 15, 55, 25 values.
I suggest as a step towards dynamic panels the defaults in the parser could be evenly distributed. (33 or rounded to 50 sounds like a sensible choice).
Maybe you could generate/display an error if a configured panel size would result in less than 1 px height and a warning in the logs if it less than 10px.

  1. dynamic module loading

I do not have a clue how this will go in Python, but I'd be surprised if this is not possible.
A (very) short search for dynamic loading/instantiation of Python modules/classes by name gives hopeful results:
https://stackoverflow.com/questions/8718885/import-module-from-string-variable
https://stackoverflow.com/questions/4821104/dynamic-instantiation-from-string-name-of-a-class-in-dynamically-imported-module
Now you get this dynamic loading, you can assert the loaded class inherits from the abstract base class and you can program to the base class only.

@Atrejoe
Copy link
Collaborator Author

Atrejoe commented May 16, 2020

VERY basic start of adding height to section config:
image

@Atrejoe
Copy link
Collaborator Author

Atrejoe commented May 17, 2020

Panel height is now generated in the settings:

python file:

ical_urls = ['https://calendar.google.com/calendar/ical/en.usa%23holiday%40group.v.calendar.google.com/public/basic.ics']
rss_feeds = ['http://feeds.bbci.co.uk/news/world/rss.xml#']
update_interval = "60"
api_key = ""
location = "Stuttgart, DE"
week_starts_on = "Monday"
calibration_hours = [0,12,18]
model = "epd_7_in_5_v2_colour"
language = "en"
units = "metric"
hours = "24"
top_section = "inkycal_weather"
top_section_height = None
middle_section = "inkycal_calendar"
middle_section_height = 77
bottom_section = "inkycal_rss"
bottom_section_height = 23
inkycal_image_path = "https://github.com/aceisace/Inky-Calendar/blob/master/Gallery/Inky-Calendar-logo.png?raw=true"
inkycal_image_path_body = ""

json(c) file:

{
	"language": "en",
	"units": "metric",
	"hours": 24,
	"model": "epd_7_in_5_v2_colour",
	"update_interval": 60,
	"calibration_hours": [
		0,
		12,
		18
	],
	"panels": [
		{
			"location": "top",
			"type": "inkycal_weather",
			"height": null,
			"config": {
				"api_key": "",
				"location": "Stuttgart, DE"
			}
		},
		{
			"location": "middle",
			"type": "inkycal_calendar",
			"height": 42,
			"config": {
				"week_starts_on": "Monday",
				"ical_urls": [
					"https://calendar.google.com/calendar/ical/en.usa%23holiday%40group.v.calendar.google.com/public/basic.ics"
				]
			}
		},
		{
			"location": "bottom",
			"type": "inkycal_rss",
			"height": 25,
			"config": {
				"rss_urls": [
					"http://feeds.bbci.co.uk/news/world/rss.xml#"
				]
			}
		}
	]
}

@aceisace
Copy link
Collaborator

@Atrejoe

Interface

OK, finally went a step ahead regarding the class inheritance and module template. About this error:

TypeError: __init__() takes 3 positional arguments but 4 were given

I've copy-pasted the code 1:1, so not modification. After looking up this site, I found the issue:

This line:
super().__init__(self, section_size, section_config)
should not have a self parameter, so it needed to be changed to:
super().__init__(section_size, section_config)

After changing this, the error is no longer happening. It's going to take some more time to use this template on all other modules, but at least this is working as expected.

  • easier config of inkycal

Another possible route would be serving a configuration-website on the device itself and then generate the file on the device (you could then even implement features like user-triggered-reloading). Also this is not one of my favorites, but I can so why some people would like it.

I like this approach more than the other options. It's easy setting up apache2 and php7 on the rpi to serve the web-ui locally. I don't know the specifics on how the web-ui can load the config from an existing settings.json file, but there are a lot of advantages with this approach.

  • module lookup by name

I think your approach does match my description. This way you could reduce the number of loaded modules ánd use the same module multiple times with different config, right? See also (5)

Yes, this is correct. Inkycal_modules cannot access other modules and therefore cannot initialise themselves. The main file, Inkycal.py (ad interim) is the one that will handle initialising all other modules (specified in settings.json).

  • section sizes

Thanks for the modifications in the web-ui. I'll work on the suggestions as soon as I'm done with the modules.

  • dynamic module loading

Thanks for the links. They seem to lead in the correct direction :). I'll play around with the code to see what is best suited for this task.

@aceisace aceisace added Beta branch (dev) Related to the dev branch (beta). Mainly for developers... devops labels May 17, 2020
@aceisace
Copy link
Collaborator

@Atrejoe
Just another quesion: The web-UI currently has to be modified manually to support as many parameters as required for each module. Some modules however, have a lot of configurable parameters.

I'm wondering if it's at all possible to have something like a dynamic web-ui, where the web-ui shows all self.x paramters of all modules the users has chosen to be displayed. That way, the full configuration could be done within the config file and it would save a lot of work on the web-ui. But I might be thinkting too far ahead...

@Atrejoe
Copy link
Collaborator Author

Atrejoe commented May 18, 2020

It certainly is póssible to make it dynamic, but I wonder if Python would be the best choice.

Usually when some something like this is to be done you would:

  • Find all classes that implement a base class (implementations of section)
  • An implementation of section would expose a type that defines the config (in .Net generics would come to mind).
  • This exposed config type would define the attributes that are configurable
  • These attributes would then be decorated (I don't know if this is possible in Python) with metadata for:
    • Client-side validation (min, max, required, regex etc etc)
    • Display (order, groups, labels, explanation)
    • Serialization

It is pretty critical for maintenance that this happens on the same codebase. So a python-running webserver in this case would be required.

@Atrejoe
Copy link
Collaborator Author

Atrejoe commented May 18, 2020

Argument validation in Python:
https://stackoverflow.com/a/2827094
And I see Python has webservers too, so I guess this cán all be achieved.

@aceisace
Copy link
Collaborator

@Atrejoe Thanks for all the info :). It seems that the way to go for this is to use django/flask in combination with the validation library.
I'll work on the necessary requirements to get this working after finishing off the modules.

@aceisace aceisace moved this from In planning to In progress in Release v2.0.0 (archived) May 20, 2020
@aceisace aceisace changed the title Remove preferred location of all modules and allow using them for all sections. Feature: dynamic modules (no location preference, no preferred position) May 21, 2020
@aceisace
Copy link
Collaborator

@Atrejoe
Finally got the abstract class template working as expected. I've modified a few things which were required. I got inkycal_rss working as a subclass of this template and will start working on the other ones.

I noted that the validation of every single class parameter will take a very long time (a lot of extra code). A lot of the other good python libraries don't validate every single argument either, but instead provide clear instructions on what the parameter requires. I'm thinking of omitting the validation (or at least postpone it for now) to speed up development.

@aceisace aceisace moved this from In progress to Implemented (in dev branch) in Release v2.0.0 (archived) May 28, 2020
@aceisace
Copy link
Collaborator

@Atrejoe I've merged more or less all the features from all different web-uis (manual merge). You don't need to merge them anymore.

For now, the web-ui in /dev_ver2_0 also supports:

  • Correct names of modules (switch from filenames to class names)
  • Display orientation (was available in /dev I think)
  • Configurable height (was available in /feature/json-settings)

Thanks for your help so far :)

aceisace added a commit that referenced this issue Jun 14, 2020
Switched from named locations to location numbers -> 1,2,3
Fixed the issue with incorrect quotes for ical- / rss- urls
@aceisace
Copy link
Collaborator

@Atrejoe Apart from the section height by percentage, ration or pixel-perfect, all other features discussed here have been implemented in release2.0.0 and BETA.
I'll be closing this issue since dynamic modules are officially supported in the development branches. Each module has a position key with an integer index, e.g. 1,2,3 specifiying it's place on the E-Paper display.
I'll be working on the official support of your module next. Thanks for your help with this issue!

@aceisace aceisace added Done! Something has been achieved! and removed devops in progress Something is currently being tested. labels Nov 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Beta branch (dev) Related to the dev branch (beta). Mainly for developers... Done! Something has been achieved!
Projects
No open projects
Release v2.0.0 (archived)
  
Implemented (in dev branch)
Development

No branches or pull requests

2 participants