Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some comments. Also, fix pylint stuff in https://travis-ci.org/alexa-pi/AlexaPi/builds/182199435, it's everything under
Module alexapi.device_platforms.magicmirror
#!/bin/bash | ||
|
||
function install_device { | ||
echo "No GPIO to install" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't display anything here.
""" | ||
|
||
class MagicmirrorPlatform(BasePlatform): | ||
__metaclass__ = ABCMeta |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not an abstract class, get rid of this.
from __future__ import print_function | ||
import time | ||
import os | ||
from abc import ABCMeta |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not an abstract class, get rid of this.
class MagicmirrorPlatform(BasePlatform): | ||
__metaclass__ = ABCMeta | ||
|
||
def __init__(self, config): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do better ordering, blank lines, etc.
Something like
+ if config['debug']:
+ print("Initialising Magic Mirro platorm")
+ super(MagicmirrorPlatform, self).__init__(config, 'magicmirror')
+ self.HOST_NAME = self._pconfig['hostname']
+ self.PORT_NUMBER = self._pconfig['port']
+ self.MM_HOST = self._pconfig['mm_hostname']
+ self.MM_PORT = self._pconfig['mm_port']
+ self.HB_TIMER = self._pconfig['hb_timer']
+ self.shutdown = False
+ self.req_record = False
+ self.httpd = ""
+ self.serverthread = ""
don't mind the +.
self.mm_heartbeat() | ||
|
||
def indicate_recording(self, state=True): | ||
if state: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using these forks doesn't look clean to me.
Use something like
self.update_mm("recording" if state else "idle")
In other places too.
def update_mm(self, status): | ||
address = ("http://" + self.MM_HOST + ":" + self.MM_PORT + "/alexapi?action=AVSSTATUS&status=" + status) | ||
try: | ||
if self._config['debug']: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be a blank line before (if there is some statement before) and after every
if self._config['debug']:
blah
This is hard to read.
|
||
if 'action' in query_dict.keys(): | ||
if (self.callback(query_dict)): | ||
self.wfile.write("{\"status\":\"success\"}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does ' work in python like in PHP so you don't have to escape the crap out of it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better code:
self.wfile.write('{"status": "success"}')
|
||
class MagicmirrorPlatform(BasePlatform): | ||
__metaclass__ = ABCMeta | ||
|
||
def __init__(self, config): | ||
print("init") | ||
if config['debug']: | ||
print("Initialising Magic Mirro platorm") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in "Mirror"
618a6b9
to
5672be9
Compare
# - Update the Magic Mirror display with AlexaPi's status (No connection, idle, listening, processing, speaking) | ||
# - Allow the Magic Mirror to trigger a 'start listening' request | ||
|
||
# pylint: disable=invalid-name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, this is cheating. Disable only for specific lines, not the whole file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, CR-wise I'm happy with it now. I can't / won't test and I'm not sure someone else will. Please do a clean install with the latest code from this branch and then I give this a go.
|
||
def __init__(self, config): | ||
if config['debug']: | ||
print("Initialising Magic Mirro platorm") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's still the typo as pointed out by lechat.
As discussed on gitter, this doesn't touch other code and is an external independent component, so I've merged it in v1.3 even though it brings new functionality. |
The allows AlexaPi to be used with the Magic Mirror, specifically the MMM-AlexaPi Module
Main project: https://github.com/MichMich/MagicMirror
AlexaPi module: https://github.com/dgonano/MMM-AlexaPi
They communicate using GET requests which update the AlexaPi status on the display (as opposed to using LEDs) and allow a method to imitate a button press.
Short video of it in action attached. It's a bit rough due to the way the platform starts and stop indicating each action but it can be smoothed out. I've noticed this delay has gotten worse with the latest additions, it was hardly noticalbe before.
https://www.icloud.com/sharedalbum/#B0T532ODWJGKIj5;57DD0F24-D727-4AE5-90A3-77B8B250236D