Super-Simple-RGB-WiFi-Lamp icon indicating copy to clipboard operation
Super-Simple-RGB-WiFi-Lamp copied to clipboard

Dynamic Modes

Open thebigpotatoe opened this issue 4 years ago • 8 comments

This branch adds the ability to dynamically add modes along the lines of #28. It adds the need for each mode to have its own HTML encapsulated in its class rather than on the main web_page.ino file. This is to allow the dynamic addition and removal of modes with the increasing contributions of others.

This is achieved by having each mode as a derived class of base mode with virtual functions to allow each mode to have a specific way of controlling the LED's and creating its part of the web-page. Each mode needs to be created and stored in a separate .ino file with a small blurb on what that modes does.

Documentation still needs to be written, but there are a few helper scripts available to help convert HTML and JS to C++ code to implement in new mode classes. If you are interested in creating a new mode and would like an example, have a look at the circle mode for something really basic, or the colour mode for a more complex example.

thebigpotatoe avatar Dec 08 '19 04:12 thebigpotatoe

I see you had some fun with Python ;).

Some thoughts about the current state of the changes:

  • Wouldn't it be a clearer API to let the sendWebsiteData return a structured object with separate attributes (modeName, tabHtml, tabScript)?
  • The JSON encoding could then be made in the generic code which would abstract and centralize the server side API for the JS frontend.
  • The escaping of the HTML for JSON could also be made ad-hoc during processing. I'd guess that this is something ArduinoJson can help.

LarsMichelsen avatar Dec 10 '19 21:12 LarsMichelsen

I'm not going to lie, that helper script is magic, so I adapted it to also transform the other parts of code too. I also had to add a little extra onto yours to help with back slashes as well if you noticed that. I also had the idea of maybe making it a website that people can copy and paste code into to make it a little easier to then paste into their mode class.

But to address your points;

  • I think it would be a great idea to return an object (or pointer) with those. As I was writing this I had the thought but persisted with what I had in the mean time to make sure it worked
  • The JSON encoding was quick and dirty, but it also does not use a lot of memory. But yes, the big thought I had was that it is an added part of complexity you have to get right when adding new modes. It should be a method of a structure to use a standardised method.
  • The escaping of the JSON actually took me the longest to figure out since there are some weird escapes that need to be done for HTML in JSON so I decided to hand write it and adapt your script to speed things up. So this is an interesting point as I did not try arduino JSON for this.

thebigpotatoe avatar Dec 13 '19 03:12 thebigpotatoe

Just experimented a bit with your changes.

I think most users will clone / download your repo and add their configuration. This is the first modification to a source controlled file. Then some users may come accross the modes in "Mode Registry" and would copy them around to the Super_Simple_RGB_WiFi_Lamp directory and modify mode_registry.ino, the second modification to a source controlled file.

With these local modifications it will be hard to track the upstream changes. Only with enough git knowledge it gets easier, because you can create a custom branch and rebase your changes at any time on the upstream changes. But that's way to advanced for the beginner.

I'd recommend to focus on a solution that goes hand in hand with a solution of #27. An approach could be to have something like a "config.h.sample", which is available in git.

The first step for every user is to copy this file to "config.h" after feching the code. Then there is either the configuration of the LED and Wifi setup, but also the declaration which modes should be included.

This is something I would find pretty easy to understand, even for the beginner. And there is no file conflict with the git contents, which makes it easy to update the code to a newer version.

What do you think?

LarsMichelsen avatar Dec 20 '19 21:12 LarsMichelsen

I like your thinking on this and the points raised in #27.

Like I stated over there, I think this is more in line with a library, and the more changes made to this repo the more I want to make that push.

Your right about tracking changes for beginners across updates, so again an Arduino library that is supported in the IDE via the lib manager I think is the best option for this. That way the IDE can be more responsible for updates rather than the user cloning or downloading too.

Through the library format we can include everything we need from logic, configs, new modes, etc while maintaining a git. This would also simplify the base sketch as well to essentially one big setup file. If we standardize the API the users sketch should be able to remain the same through updates until they want to change things.

What I would like to do for this pull is figure out how we need to create and store the modes so that it makes the most sense to anyone willing to develop their own. The structures in which this info is held should also be easy to create yet be efficient in storage and memory, as well as easily maintainable. You mention half of this above, all of which are great ideas which should be implemented.

After this pull I feel we should move to a library after tidying other pull requests as well. What are you thoughts?

thebigpotatoe avatar Dec 30 '19 00:12 thebigpotatoe

Sounds like a good idea. I haven't worked with Arduino libraries before. But from what I can read at https://www.arduino.cc/en/Hacking/libraryTutorial it is pretty simple to create a simple library.

Seems to be a good idea to first merge the pending PRs, because the library would involve a lot of restructuring which then results in some kind of merge barrier.

Regarding the structuring of the modes: Considering that the git is to be split and the library created from it, it should contain standard modes that just work.

Also, the user should be able to create and activate his own modes. Since all modes are constructed in the same way, he can also upload them via PR.

Requirements I see

  1. all modes have the same structure
  2. modes are declarative and have as few dependencies to the library code as possible
  3. there are built-in standard modes
  4. all standard modes are active by default
  5. as user I can create and activate my own modes
  6. as a user I can also activate and deactivate built-in modes by configuration
  7. it is documented how modes can be sent as PR

Come to think of it, your adaptations in the new fashions industry go in that direction. The default modes would still be in the library code (currently: Super_Simple_RGB_WiFi_Lamp/Mode*). And the user can create his own mode classes in his sketch and then include them and register them in "modes" using his configuration.

I think an interesting question might be: How does a user sketch look like then? Some quick idea:

#include <SuperSimpleRGBWiFiLamp.h>
#include "ModeMyFancyThing.h"

SuperSimpleRGBWiFiLamp lamp = Lamp();

lamp.data_pin = D1;
lamp.num_leds = 100;
lamp.utc_offset = +10;
lamp.top_leds = {...};
lamp.bottom_leds = {...};
lamp.left_leds = {...};
lamp.right_leds = {...};

lamp.addCustomMode("my_fancy_thing", new ModeMyFancyThing());

void setup() {
    lamp->setup();
}

void loop() {
    lamp->loop();
}

LarsMichelsen avatar Dec 30 '19 18:12 LarsMichelsen

I have made a bunch of private Arduino libraries for my own stuff. Its dirt simple and allows you to have a structure behind your project away from how the IDE compiles the .ino files.

I just uploaded my first public library today (awaiting response), but this can give you the idea. I would hope that pushing this repo down the library path that we end up with something similar which is available from the IDE library manager.

Also, the user should be able to create and activate his own modes. Since all modes are constructed in the same way, he can also upload them via PR.

This is an absolute must have. Perhaps if we segregate the mode class you created to a separate space so that it could be publicly inherited by anyone. The use of private virtuals correctly should help people set it up correctly, and give errors at compile time.

#include <SuperSimpleRGBWiFiLamp.h>

class myNewMode : public SuperSimpleRGBWiFiLamp::modeClass { ... }

myNewMode newMode;

void setup() {
    lamp->setup();
    lamp->addNewMode(&newMode)
}

void loop() {
    lamp->loop();
}

The default modes would still be in the library code (currently: Super_Simple_RGB_WiFi_Lamp/Mode*). And the user can create his own mode classes in his sketch and then include them and register them in "modes" using his configuration.

My ideas for my branch were trying to go in this direction, the separate folder for new modes makes sense as a library, but not as a sketch. I think we could make it simple for the user and eliminate unnecessary code if we used pre-processor directives for this:

#include <SuperSimpleRGBWiFiLamp.h>

// To exclude default mode 
#define EXCLUDE_RAINBOW

// To include user contributed modes
#define INCLUDE_SPARKLE

SuperSimpleRGBWiFiLamp lamp = Lamp();

...

thebigpotatoe avatar Dec 31 '19 05:12 thebigpotatoe

If the preprocessor macros work for all our use cases, that's fine. The macros have some advantages, like a) they exclude the whole code at compile time b) they allow the compiler to fail in case there is something broken when excluding it.

Compared to the explicit config attributes, a downside is that there is no central place where you can get a view on all possible settings (macros) of the lamp. In the program code they are similar to global variables with all their downsides. From a top level view you can not see which setting is inherited into which part of the code, you can only search for all occurrences and try to understand the structure / relations. I understand that this is a small project and these things do not have much effect on the comprehensibility of the code. So the macros are OK for me.

LarsMichelsen avatar Dec 31 '19 13:12 LarsMichelsen

Totally agree with your statements. Will just have to write some code and see what fits best. But you are right. The management of macros is not fun, so I think the aim will be for config attributes as you say.

thebigpotatoe avatar Jan 02 '20 06:01 thebigpotatoe