MagicMirrorOrg / MagicMirror

MagicMirror² is an open source modular smart mirror platform. With a growing list of installable modules, the MagicMirror² allows you to convert your hallway or bathroom mirror into your personal assistant.
http://magicmirror.builders
MIT License
19.33k stars 4.15k forks source link

New Module System Discussion - Please contribute. #80

Closed MichMich closed 8 years ago

MichMich commented 8 years ago

Maybe it's a good idea to discuss the new Module System before we writeout the API.

@paviro, I noticed you've contributed. Thanks for this. I really appreciate your help! Just a few questions though:

The functions you proposed, are those method of a module class? I can't really wrap my head around how this would work, could you elaborate?

Also, I think the added regions are a bit unnecessary. In most cases you can just add content in one of the corners (the content will stack) or align right or left in the upper third, center or lower third. I think we should try to keep the layout simple.

Aslo, what do you guys think about using a system like vue.js? I'm not a big fan of jQuery, and prefer a mvvm model. But this might make it to difficult for most users to contribute.

EDIT 2016/3/23: First steps in the development are made. See: https://github.com/MichMich/MagicMirror/issues/80#issuecomment-200411278

MichMich commented 8 years ago

I thought about it, but since a module can be included more then once, it has no use to push the config of the client to the node_helper. None the less, you should be label to just use the config variable (i think it's available in for the helper as well since it's a global ...), best practice would be to send the configuration from a module using the sockets when you perform a socket request.

paviro commented 8 years ago

Valid point but I think modules that don't offer any direct UI and also don't need a position (like my call monitor and the face reg) should not be able load more than once anyway, would not make any sense or is there any case where it would? Maybe allow a module to specify that it can only be loaded once? It does not seem like I can access it via this.config just returns undefined. It would be great if it would be possible, since the module will use a python script and I want to allow the user to set the settings via config.js and not force him to edit the node helper or even the Python script.

MichMich commented 8 years ago

You could try just config (without this.).

I think the best approach would be:

modulename.js

start: function() {
    sendNotification: function('CONFIG', this.config);
}

node_helper.js

socketNotificationReceived: function(notification, payload) {
     if (notification === 'CONFIG') {
         this.config = payload;
         //start setup of the python object.
    }
}
paviro commented 8 years ago

I can't figure out what I am doing wrong... I used sendNotification: function('CONFIG', this.config); but than I got an errorUncaught SyntaxError: Unexpected token (. If I do this.sendSocketNotification('CONFIG', this.config); nothing ever gets received in the node helper..

MichMich commented 8 years ago

Could you post your code somewhere?

paviro commented 8 years ago

Here you go.

MichMich commented 8 years ago

It looks ok. Can't really check it since I am on my phone. Did you pull the latest v2 version?

MichMich commented 8 years ago

Also, why use the timer? You could start python when the config arrives. (IF it arrives. ;))

paviro commented 8 years ago

I used the timer because I was scared that it could be started twice if someone else sends something as CONFIG... I updated now, have you changed how the helpers are called? Because I now get No helper found for module: MMM-FRITZ-Box-Callmonitor.. Oh sorry.. Git somehow messed up the modules and deleted the content...

paviro commented 8 years ago

Works now, updating did the trick, thanks!

MichMich commented 8 years ago

So it's solved?

Also, do something like:

If(!pythonStarted) { pythonStarted=true; pythonStart(); }

paviro commented 8 years ago

That makes sense! Is it correct that I can't access this.name within defaults? I am trying to set the default path to the training file and therefore add the modules name to the path but it seems to be undefined.

MichMich commented 8 years ago

Correct. It's not set yet when instanciating the module. Going to bed now. Will continue further dev tomorrow.

paviro commented 8 years ago

Alright then I'll hardcode it for now. Sleep well :) Update (02:20 UTC+2): I am done, looking forward to add the module swapping! It would be great if there was a way to list all available module classes that can be hidden/shown, so that I can check if a recognized person has its own set of modules or if I should just say Hi via an alert and leave on the default modules.

MichMich commented 8 years ago

I made a breaking change to the module loader. Modules should now be registered using the following method:

Old way:

Module.create({
 ...
});

New way:

Module.register('helloworld',{
 ...
});
paviro commented 8 years ago

Updated my modules. What is the advantage internally of loading them this way?

MichMich commented 8 years ago

Easier debugging. And no need for tricky eval() method.

I'm currently experiencing an issue though. One module won't be started. Currently investigating it.

MichMich commented 8 years ago

Issue is fixed.

paviro commented 8 years ago

Any ETA on the module swapping?

MichMich commented 8 years ago

Working on it as we speak ... ;)

MichMich commented 8 years ago

Ok, so, the module swapping should be working now ... It needs some documentation, so here you go.

To make a selection of modules, you have a lot of options:

// All modules:
var module = MM.getModules();

// All modules with specific classes:
var module = MM.getModules().withClass('classname');
var module = MM.getModules().withClass('classname1 classname2');
var module = MM.getModules().withClass(['classname1','classname2']);

// All modules except with specific classes:
var module = MM.getModules().exceptWithClass('classname');
var module = MM.getModules().exceptWithClass('classname1 classname2');
var module = MM.getModules().exceptWithClass(['classname1','classname2']);

// All modules except a specific module:
var module = MM.getModules().exceptModule(aModule);

// You can combine the methods above:
var module = MM.getModules().withClass('classname1').exceptwithClass('classname2').exceptModule(aModule);

If you want to perform an action with those methods, you can use the enumerate function:

MM.getModules().enumerate(function(module) {
    Log.log(module.name);
});

Hiding/showing a module is simple:

    //Hide with last used speed.
    module.hide();

    //Hide with 1 second animation.
    module.hide(1000);

    //Hide with 1 second animation and callback.
    module.hide(1000, function() {
        Log.log('Module is hidden.');
    });

    //Show with last used speed.
    module.show();

    //Show with 1 second animation.
    module.show(1000);

    //Show with 1 second animation and callback.
    module.show(1000, function() {
        Log.log('Module is shown.');
    });

And of course, you can combine all of the above:

MM.getModules().withClass('classname1').exceptwithClass('classname2').exceptModule(aModule).enumerate(function(module) {
    module.hide(1000, function() {
        Log.log('Module is hidden.');
        module.show(1000, function() {
            Log.log('Module is shown.');
        });
    });
});

Last but not least, if you want to add a class to a specific module in the config, this can be done using the classes property:

{
    module: 'modulename',
    position: 'lower_third',
    classes: 'classname1 classname2 classname3'
}

Note 1: The system does not invoke a suspend method on the module yet. Note 2: The method MM.getModules() returns an empty array if not all modules are started yet. If you want to make sure all modules are started, you can listen for a system notification.

notificationReceived: function(notification, payload, sender) {
    if (notification === 'ALL_MODULES_STARTED') {
        // ... do something with here MM.getModules()
    }
}

Let me know how this system works for you.

paviro commented 8 years ago

Great nice! I just tried to write a Wunderlist module but am a bit stuck there so I will implement this one now :) I hate asynchronous code (at least right now)...

MichMich commented 8 years ago

Lol. I understand what you mean. :)

paviro commented 8 years ago

Mhh seems like I am doing something wrong... Uncaught TypeError: MM.getModules(...).withClass(...).exceptwithClass is not a function Just copy and pasted what you posted here.

MichMich commented 8 years ago

exceptWithClass (capital W)

paviro commented 8 years ago

Seems to work! Nice! Only thing, I am using this code to hide any module that does not have the class "default", but it does not do anything. if (notification === 'ALL_MODULES_STARTED') { MM.getModules().exceptWithClass('default').enumerate(function(module) { module.hide(0, function() { Log.log('Module is hidden.'); }); }); }

Oh and it seems like iCloud calendars do not work, just keeps loading.

MichMich commented 8 years ago

Above code looks ok and iCloud agenda works here. Ill look into tomorrow if you can't solve it.

MichMich commented 8 years ago

I suggest you do some logging with your above code to see which modules it selects.

SVendittelli commented 8 years ago

The calendar only seems to load the first of recurring events and only if that event is in the future, see example below

image screenshot 2016-03-31 20 30 33

paviro commented 8 years ago

I do, just added console.log(module); and every module is logged just not faded out.. I'll check my module.hide again.

MichMich commented 8 years ago

@SVendittelli thanks! Could you make a separate issue for this? I'll look into it tomorrow.

SVendittelli commented 8 years ago

Sure thing!

paviro commented 8 years ago

Could not find the problem yet but I already noticed one thing. Hidden modules take up space on the UI, so if you would for example have three sets of modules for three people that would not work...

MichMich commented 8 years ago

Will look into it tomorrow. Could you create a simple module with the non working code so I can test?

paviro commented 8 years ago

Sure, here you go :)

MichMich commented 8 years ago

Hidden modules take up space on the UI, so if you would for example have three sets of modules for three people that would not work...

This should be fixed now.

paviro commented 8 years ago

Nice, works thanks! Weirdly the fade in animation seems not to work anymore, modules just appear and then jump to the new position.

MichMich commented 8 years ago

Is working here ... REALLY going to bed now. ;) I'll check out the issues tomorrow.

paviro commented 8 years ago

Any idea why this module overlaps with the calendar? Screenshot

MichMich commented 8 years ago

Are they in the same position? If so please open a separate issue with example code.

MichMich commented 8 years ago

Found the issue. It's fixed. As well as the fading that didn't work at all times.

paviro commented 8 years ago

Nice!

MichMich commented 8 years ago

I made a small start with the module documentation: https://github.com/MichMich/MagicMirror/tree/v2-beta/modules/clock https://github.com/MichMich/MagicMirror/tree/v2-beta/modules/calendar

I will continue to add documentation of the other models on a later stage. If anyone feels the urge to document the other modules, feel free! ;)

MichMich commented 8 years ago

All module README.md's are ready. :)

CFenner commented 8 years ago

Is the module system in a 'nearly complete' state by now?

MichMich commented 8 years ago

Can't promise anything ... ;)

I'm working on one major change now. But that won't be a breaking change. After that, the next step will be the full documentation.

paviro commented 8 years ago

Do you have time to add the UI notifications or should I give it try this evening? :)

Am 01.04.2016 um 17:07 schrieb Michael Teeuw notifications@github.com:

Can't promise anything ... ;)

I'm working on one major change now. But that won't be a breaking change. After that, the next step will be the full documentation.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub

MichMich commented 8 years ago

No, sorry. No time for it. You can try, but I can't promise I will include/use it. Make sure you just make a separate module for it. (No core changes should be necessary);

MichMich commented 8 years ago

I made a few huge changes on the loader. You should now be able to put modules in folder. In the config you should be able to use module: 'foldername/modulename'. The default modules don't need the default/ prefix.

This way you can make packages of multiple modules.

CFenner commented 8 years ago

Do I need to wrap a module in an extra folder or does the old way still work?