Callmonitor für die FritzBox

Stellt hier eure Projekte vor.
Internetseiten, Skripte, und alles andere bzgl. Python.
maxi_king_333
User
Beiträge: 110
Registriert: Freitag 25. Dezember 2009, 03:42

Dauerbaustelle hat geschrieben:Ich aber nicht! Und ich möchte den Code vielleicht auch lesen und verstehen. Das Problem mit der Liste, von der keiner weiß, was die Elemente bedeuten, wäre ja gar nicht so gravierend, wenn die Liste nicht an verschiedenen Stellen verwendet würde, weil man dann einfach

Code: Alles auswählen

gutebeschreibung1, gutebeschreibung2, ... = foo.split(';')
machen könnte. Deine seltsame Liste wird aber rumgereicht und man muss für das Verstehen einer Codestelle, in der `call[n]` auftritt, genau verstehen, wie `call` zustande kommt. Und wenn es dann auch noch im Laufe des Programms umbenannt wird, wirds ziemlich hoffnungslos :-)
Das ist aber so leider nicht möglich, da ich gutebeschreibung2 kennen muss um die restlichen Namen zu haben.
Daher brauche ich wohl oder übel die "magischen Zahlen".
Außer es gäbe eine magische Python-Funktion. (vielleicht gibt es die ja sogar)
Rumgereicht wird die Liste auch nicht, sie wird nur an ein Objekt von Call übergeben, das die Liste in ein Dictionary(sich selbst) umwandelt, welches dann rumgeht.
Und das es umbenannt wird, stimmt so nicht.
Es heißt überall call, wobei call ein Objekt der Klasse Call ist bzw. temporär das Ergebnis von split(';').
Damit man sieht, das das Objekt von Call und der gesplittete String zusammengehören.
EyDu hat geschrieben:Ach ja, da werden Erinnerungen wach. Die Erfahrung von magischen Zahlen, deren Bedeutung man mit absuluter Sicherheit niemals vergisst, haben hier sicher alle schon gemacht. Du in wenigen Wochen wahrscheinlich auch ^^
Wie gesagt, der Code mit den magischen Nummern ist über ein Jahr alt.
Die Nummern sind ja auch nicht irgendwie willkürlich nur um irgendwelche Programm-Internen-Daten zu speichern, sondern sie kommen von Außen.
Wäre es Programm-Intern und ich könnte sie beliebig verändern, dan würde ich natürlich keine Nummern nehmen.

Nichts nichtsdestotrotz habe ich mal 2 Kommentare, die die Struktur erklären eingefügt.
Würde das gerne auch ohne die "magischen Nummern" lösen. - Weiß aber nicht wie...
Dauerbaustelle
User
Beiträge: 996
Registriert: Mittwoch 9. Januar 2008, 13:48

Okay das Protokoll ist echt ein wenig... alternativ.

Hier mal ein Vorschlag. Die definierst erstmal eine abstrakte Datenstruktur für Anrufe.

Code: Alles auswählen

class BaseCall(object):
  def __init__(self, from_, to, contacts=contacts.Contacts()):
    self.from_ = from_
    self.to = to
    self.active = False
    self.contacts = contacts

  def get_from_name(self):
    return self.contacts.resolve(self.from_)

  def get_to_name(self):
    return self.contacts.resolve(self.to)

  def on_connect(self):
    self.active = True

  def on_disconnect(self, duration):
    self.active = False
    self.duration = duration
Dann jeweils eine für ausgehende und ankommende.

Code: Alles auswählen

class IncomingCall(BaseCall):
  def __init__(self, from_, to, over_intern):
    super(IncomingCall, self).__init__(from_, to)
    self.over_intern = over_intern

class OutgoingCall(BaseCall):
  def __init__(self, from_, to, over_extern):
    super(OutgoingCall, self).__init__(from_, to)
    self.over_extern = over_extern
Mehr brauchst du in der Datenstruktur erstmal nicht. Die Parsing-Sachen einfach in Funktionen reinstopfen, die haben nicht wirklich was mit der Datenstruktur eines Anrufs zu tun (bin mir da aber nicht 100% sicher :-).

Jetzt zum richtigen Spaß. Dieses Protokoll. Das Auspacken des `split`-Ergebnisses wäre in Python 3 schöner als in Python 2:

Code: Alles auswählen

timestamp, event, *data = line.split(';')
Nun geht das aber nicht in Python 2, deswegen hier der Workaround:

Code: Alles auswählen

data = line.split(';')
timestamp = line.pop(0)
event = line.pop(0)
(Wer hier ne schönere Idee hat, immer her damit!)

Jetzt weiß man schonmal, welche Bedeutung das erste und das zweite Element der Liste haben. Jetzt das muss man das ja auch noch dispatchen:

Code: Alles auswählen

handler = getattr(self, 'handle_%s' % event)
handler(*data)
Das ruft z.B. bei einem "connect" die Methode `handle_connect` mit den Parametern in `data` auf.

Und dann braucht man auch noch Implementation für die ganzen Events:

Code: Alles auswählen

def handle_call(self, id, over_intern, from_, to):
  self._calls[id] = IncomingCall(from_, to, over_intern)
  self._invokel_callbacks('call', id)

def handle_ring(self, id, from_, to, over_extern):
  self._calls[id] = OutgoingCall(from_, to, over_extern)
  self._invoke_callbacks('ring', id)

def handle_connect(self, id, over_intern):
  self._calls[id].on_connect()
  self._invoke_callbacks('connect', id)

def handle_disconnect(self, id, duration):
  self._calls[id].on_disconnect(duration)
  self._invoke_callbacks('disconnect', id)
  del self._calls[id]
(Ich habe grade keine gute Idee, wie man das ständige `_invoke_callbacks` loswerden kann, wegen dem `del` bei Disconnects.)

Hoffe, dass inspiriert. :-)
maxi_king_333
User
Beiträge: 110
Registriert: Freitag 25. Dezember 2009, 03:42

Hi,

sorry, dass ich mich jetzt erst melde, hatte mir durch ein Update auf Gnome 3 das System zerschossen.
In der Tat, war das eine Inspiration und es ist auch etwas bei rumgekommen:
https://github.com/koehlma/fbcallnotify ... monitor.py
Nur ist mit das irgendwie zu kompliziert, mit den Vererbungen, hasattr und getattr, jedoch weiß man jetzt, was was ist.
Da kam mir die Idee das ganze mit Regular Expression zu lösen, mich würde mal interressieren, was Ihr (Du) davon haltet.
Kann das ja durch das Versions-Management jederzeit rückgängig machen.
Ich finde diese Lösung sehr gut:
Man weiß was was ist, durch ein Dictionary.
Keine eigenen Daten-Typen mehr.
Updaten der Informationen durch die Dictionary eigene update()-Funktion.

Viele Grüße
Maxi
Dauerbaustelle
User
Beiträge: 996
Registriert: Mittwoch 9. Januar 2008, 13:48

Das mit den Regular Expressions finde ich ziemlich dämlich. Du gewinnst dadurch gar nix außer Unleserlichkeit. Das Datenformat ist so simpel, dass ein manueller "Parser" (der ja im Wesentlichen aus einem `.split(';')` besteht) viel verständlicher ist. Die Regular Expressions sind so vollgestopft von "Rauschen", dass man nicht erkennen kann, welches Format die Eingangsdaten überhaupt haben.

Welche Bedeutung hat eigentlich `Callmonitor._functions`? Konnte das nirgendwo sonst finden, und die Bedeutung wird dank nichtssagendem Namen auch nicht klar.

Übrigens ist durch das geregexe die `handle_read`-Methode wieder total unübersichtlich geworden.

`add_callbacks` und `add_actions` sehen mir verdächtig ähnlich, vielleicht kann man davon eines eliminieren oder alternativ `add_action(filter, action)` definieren durch `add_callbacks(filter, action.ring, ... action.disconnect)`.

Methoden würde ich übrigens wie folgt anordnen: In "Einheiten" bündeln und innerhalb dieser Einheit nach logischer Abfolge/Abhängigkeit. Also z.B. `main` und dann `main_quit` direkt nach `__init__`, und `handle_close` nach `handle_read`.

Zur Kontakt-Datenbank: Die "späte" Initialisierung würde ich ganz rausnehmen und dieses bisschen Code auf Modulebene stattfinden lassen. Das `__init__` einer Klasse sollte nichts tun als den Zustand des neu erstellen Objektes initialisieren, also Attribute setzen und so fort. Das heißt aber, dass dort _keine_ Logik stattfinden soll, und schon gar nicht solche, die Seiteneffekte hat (Dateien lesen). Das würde ich in eine Methode auslagern und die dann von mir aus direkt nach Erstellen des Objektes aufrufen.
maxi_king_333
User
Beiträge: 110
Registriert: Freitag 25. Dezember 2009, 03:42

Hi,
Das mit den Regular Expressions finde ich ziemlich dämlich. Du gewinnst dadurch gar nix außer Unleserlichkeit. Das Datenformat ist so simpel, dass ein manueller "Parser" (der ja im Wesentlichen aus einem `.split(';')` besteht) viel verständlicher ist. Die Regular Expressions sind so vollgestopft von "Rauschen", dass man nicht erkennen kann, welches Format die Eingangsdaten überhaupt haben.
Hmm...
Ich finde das mit den Funktionen und den Parametern irgendwie unleserlich (mag daran liegen, dass ich noch nicht soviel Ahnung habe).
Wobei ich sagen muss, das mit den Regular Expressions ist auch nicht das gelbe vom Ei.
Es gab schonmal ein ähnliches Projekt, das allerdings die Notifications für alle Nummern anzeigt.
Die haben das so gelöst: http://pynogio.cpegel.de/trac/browser/pynogio/event.py
Dieses Event Objekt wird jedesmal erzeugt und dann hier von evaluate verarbeitet: http://pynogio.cpegel.de/trac/browser/p ... manager.py
Dort wird auch mit den magischen Nummern gearbeitet, wobei das Zuordnen in der gleichen Methode wie das Splitten passiert.
Was hällst Du davon?
Ansonsten werde ich mich wohl an die Parameter gewöhnen müssen. ;)
Zur Kontakt-Datenbank: Die "späte" Initialisierung würde ich ganz rausnehmen und dieses bisschen Code auf Modulebene stattfinden lassen.
Verstehe ich leider nicht ganz, soll ich also aus dem ganzen nur Funktionen auf Modulebene machen?

Die anderen Dinge habe ich ausgebessert.

Vielen Dank für die Geduld und Hilfe,
Maxi
Dauerbaustelle
User
Beiträge: 996
Registriert: Mittwoch 9. Januar 2008, 13:48

maxi_king_333 hat geschrieben:Ich finde das mit den Funktionen und den Parametern irgendwie unleserlich
Also wenn's an der `getattr`-Magie liegt, die könntest du auch mit einem Methoden-Lookup-Dictionary ersetzen, das etwa so aussieht:

Code: Alles auswählen

{'ring' : on_ring, 'call' : on_call, ...}
Zur Kontakt-Datenbank: Die "späte" Initialisierung würde ich ganz rausnehmen und dieses bisschen Code auf Modulebene stattfinden lassen.
Verstehe ich leider nicht ganz, soll ich also aus dem ganzen nur Funktionen auf Modulebene machen?
Du hast ja da diese `init_irgendwas`. Nimm den Code und lagere ihn auf Modulebene aus, wodurch die `global`s wegfallen.
maxi_king_333
User
Beiträge: 110
Registriert: Freitag 25. Dezember 2009, 03:42

Also wenn's an der `getattr`-Magie liegt, die könntest du auch mit einem Methoden-Lookup-Dictionary ersetzen, das etwa so aussieht:
Es liegt eher an meiner Logik.
Bei den magischen Zahlen, ist sie folgende:

Code: Alles auswählen

Wenn Feld 1 xyz ist, dann... Zuordnen...
Bei den RegEx:

Code: Alles auswählen

Wenn Feld 1 xyz ist, dann... Dictionary erzeugen/updaten...
Bei den Methoden:

Code: Alles auswählen

Wenn Event xyz ist... dann Methode aufrufen... 
    Dann:
        Incoming oder Outgoing Call mit Paramtern erzeugen...
        Zuordnen...
    Oder:
       connect/disconnect von Call mit Parametern aufrufen...
       Zuordnen...
Will heißen, keine direkte Relation zwischen den Feldern und den Schlüsseln (Namen) im späteren Call-Objekt.
Du hast ja da diese `init_irgendwas`. Nimm den Code und lagere ihn auf Modulebene aus, wodurch die `global`s wegfallen.
Dann müsste ich doch aber die Vorwahl und die Landesvorwahl hart kodieren, oder wie jetzt?
Kann man an ein Modul etwa Parameter übergeben?
Dauerbaustelle
User
Beiträge: 996
Registriert: Mittwoch 9. Januar 2008, 13:48

maxi_king_333 hat geschrieben:

Code: Alles auswählen

Wenn Event xyz ist... dann Methode aufrufen... 
    Dann:
        Incoming oder Outgoing Call mit Paramtern erzeugen...
        Zuordnen...
    Oder:
       connect/disconnect von Call mit Parametern aufrufen...
       Zuordnen...
Will heißen, keine direkte Relation zwischen den Feldern und den Schlüsseln (Namen) im späteren Call-Objekt.
Versteh' ich nicht. Wie wäre es hiermit:

Code: Alles auswählen

...
def on_call(...):
  ...
...
event_handlers = {
  'call' : on_call,
  ...
}

def handle_event(self, event, *args):
  handler = self.event_handlers[event]
  handler(*args)
Du hast ja da diese `init_irgendwas`. Nimm den Code und lagere ihn auf Modulebene aus, wodurch die `global`s wegfallen.
Dann müsste ich doch aber die Vorwahl und die Landesvorwahl hart kodieren, oder wie jetzt?
Kann man an ein Modul etwa Parameter übergeben?
Hm. Das scheint mir ein typischer Fall von "overengineered" zu sein. Die Möglichkeit, andere Vorwahlen etc zu nutzen, benutzt faktisch kein Mensch. Es nützt also genau nichts, sie drin zu haben. Etwas mal einzubauen, weil man es vielleicht möglicherweise irgendwann in ähnlicher Form gebrauchen könnte, ist sinnlos und aufwändig. Meiner Meinung nach ist Software dann perfekt, wenn man nix mehr wegnehmen (darin eingeschlossen: vereinfachen) kann -- alles so simpel wie irgendwie möglich, aber so kompliziert wie nötig.

Ich würde die Einstellung der Vorwahlen höchstens in einer ganz einfachen Konfigurationsdatei anbieten, oder als Parameter der Contacts-Klasse machen.

Noch zwei Dinge: `__init__` hat noch immer Seiteneffekte, ich würde das `read` nicht innerhalb von `__init__` aufrufen, ist aber ne Geschmacksfrage denke ich.

`call_match` ist kein sonderlich guter Name und `functions.py` empfinde ich als überflüssig. Man hat normalerweise ein `utils`-Modul, wo allermöglicher Krams reinkommt, der modulübergreifend benutzt wird. Das trifft aber auf keine der beiden Funktionen da drin zu. Ich würde empfehlen, das Zeuch einfach in die Modul zu stecken, in die es "thematisch" gehört. Und `call_match` einen besseren Namen geben, der suggeriert, dass das Resultat der Funktion ein Boolean ist. Vielleicht könnte man die Funktionalität auch zur Call-Klasse hinzufügen und dann z.B. `matches_filter(s)` nennen. Oder so ähnlich.
maxi_king_333
User
Beiträge: 110
Registriert: Freitag 25. Dezember 2009, 03:42

Dauerbaustelle hat geschrieben:Versteh' ich nicht. Wie wäre es hiermit:
Naja, ist schwer zu erklären, ich finde es einfach irgendwie unleserlich und nicht so schön, wie bei Kunst. ;)
Ich werde trotzdem auf den Rat von Dir hören und meine ästhetischen Bedürfnisse mal vergessen.
Dauerbaustelle hat geschrieben:Hm. Das scheint mir ein typischer Fall von "overengineered" zu sein. Die Möglichkeit, andere Vorwahlen etc zu nutzen, benutzt faktisch kein Mensch. Es nützt also genau nichts, sie drin zu haben. Etwas mal einzubauen, weil man es vielleicht möglicherweise irgendwann in ähnlicher Form gebrauchen könnte, ist sinnlos und aufwändig. Meiner Meinung nach ist Software dann perfekt, wenn man nix mehr wegnehmen (darin eingeschlossen: vereinfachen) kann -- alles so simpel wie irgendwie möglich, aber so kompliziert wie nötig.
Ähm... Ich weiß ja nicht, aber ich schätze, dass wenn das Programm jemand anders benutzt, der nicht zufällig im gleichen Kaff wohnt wie ich, dann hat er ein Problem.
Dauerbaustelle hat geschrieben:Ich würde die Einstellung der Vorwahlen höchstens in einer ganz einfachen Konfigurationsdatei anbieten, oder als Parameter der Contacts-Klasse machen.
Noch extra eine Konfigurations-Datei, das finde ich, ist overkill.
Als Parameter für die Contacts-Klasse macht das auch keinen Sinn, da das Modul nicht gleichzeitig an zwei verschiedenen Standorten geladen wird.

Vielleicht hast Du nicht ganz verstanden, was das mit den Vorwahlen soll:
Wenn Du mich anrufst, erhalte ich (über den Callmonitor) Deine komplette Nummer (mit Vorwahl), egal ob Du im selben Kaff wie ich oder in einem anderen wohnst.
Wenn ich Dich aber anrufe, dann kann ich, sofern Du im selben Kaff wie ich wohnst die Vorwahl wählen oder auch nicht.
Daher brauche ich die Vorwahl, da der Callmonitor die Nummer immer wie gewählt übermittelt und ich nicht extra 2 Einträge (mit und ohne Vorwahl) anlegen will.
Das macht die ganze Sache ja auch so kompliziert, 1 Nummer kann ich in 4 Nummern ausdrücken.
Dauerbaustelle hat geschrieben:Noch zwei Dinge: `__init__` hat noch immer Seiteneffekte, ich würde das `read` nicht innerhalb von `__init__` aufrufen, ist aber ne Geschmacksfrage denke ich.
Also ich meine schon, dass die Datenbank direkt beim erstellen gebaut werden muss, da das Objekt fest einer Datenbank zugeordnet ist.
Dauerbaustelle hat geschrieben:Vielleicht könnte man die Funktionalität auch zur Call-Klasse hinzufügen und dann z.B. `matches_filter(s)` nennen. Oder so ähnlich.
Dauerbaustelle hat geschrieben:Mehr brauchst du in der Datenstruktur erstmal nicht. Die Parsing-Sachen einfach in Funktionen reinstopfen, die haben nicht wirklich was mit der Datenstruktur eines Anrufs zu tun (bin mir da aber nicht 100% sicher :-).
Passt nicht so gut zusammen ;). - Außer Du hast mit Parsing-Sachen nur den Singular gemeint.
Vorher hatte ich das da drin, duration_as_hms gehört ja eigentlich auch dazu.
Dauerbaustelle
User
Beiträge: 996
Registriert: Mittwoch 9. Januar 2008, 13:48

maxi_king_333 hat geschrieben:Ähm... Ich weiß ja nicht, aber ich schätze, dass wenn das Programm jemand anders benutzt, der nicht zufällig im gleichen Kaff wohnt wie ich, dann hat er ein Problem.
Hm. Das `init_numbers`, von wo wird das überhaupt aufgerufen? Von einem anderen Tool? Vielleicht wäre es sinnvoller, innerhalb von fbcallnotify nur mit absoluten (also mit Landes- und Ortsvorwahl) Nummern zu arbeiten und im Frontend auch "relative" zu erlauben?
Dauerbaustelle hat geschrieben:Vielleicht könnte man die Funktionalität auch zur Call-Klasse hinzufügen und dann z.B. `matches_filter(s)` nennen. Oder so ähnlich.
Dauerbaustelle hat geschrieben:Mehr brauchst du in der Datenstruktur erstmal nicht. Die Parsing-Sachen einfach in Funktionen reinstopfen, die haben nicht wirklich was mit der Datenstruktur eines Anrufs zu tun (bin mir da aber nicht 100% sicher :-).
Passt nicht so gut zusammen ;). - Außer Du hast mit Parsing-Sachen nur den Singular gemeint.
Vorher hatte ich das da drin, duration_as_hms gehört ja eigentlich auch dazu.
Bei diesem Filter-Matching war ich mir nicht ganz sicher, wo das hinsoll. Ich denke mal, es ist auch in Ordnung, das der Klasse hinzuzufügen. Auf keinen Fall jedenfalls in ein eigenes Utilities-Modul, weil es ja nur einen einzigen Einsatzzweck hat. Utilities nur für Kram, der universell ist. Ein ganz guter Indikator für "universell" ist, ob man den Sinn des Codes versteht, ohne den anderen Code zu kennen. `duration_as_hms` ist eine solch universelle Funktion, könnte damit also in die Utils; aber weil das nur eine einzelne Funktion ist, würde ich die einfach da reinstecken, wo's thematisch am besten passt.
Dauerbaustelle
User
Beiträge: 996
Registriert: Mittwoch 9. Januar 2008, 13:48

Was mir grade noch auffällt: Auf der Projektwebsite steht Python3, der Code ist aber Python2 (zumindest die `print`-Syntax ist nicht Python3-kompatibel). Wenn du da wirklich Python3 verwendest, kannst du für das Auspacken der Parameter von der FritzBox die neue Unpack-Syntax nutzen (wie in nem früheren Post beschrieben).
maxi_king_333
User
Beiträge: 110
Registriert: Freitag 25. Dezember 2009, 03:42

Dauerbaustelle hat geschrieben:Hm. Das `init_numbers`, von wo wird das überhaupt aufgerufen? Von einem anderen Tool?
FBCallNotify ist als eine Bibliotek zu verstehen, mit der man seinen eigenen Callmonitor aufbauen kann.
Mit eigenen Filtern und eigenen Reaktionen auf Anrufe und co.
Das contacts Modul erlaubt es Kontaktdatenbanken zu benutzen.
Ich habe also in ~/bin meinen eigentlichen Callmonitor.
Dieser benutzt z.B. auch die Banshee-Klasse.
Vielleicht wäre es sinnvoller, innerhalb von fbcallnotify nur mit absoluten (also mit Landes- und Ortsvorwahl) Nummern zu arbeiten und im Frontend auch "relative" zu erlauben?
Genau das kann ich eben nicht machen.
Da von der FritzBox die gewählte Nummer so wie gewählt übermittelt wird, arbeitet die Contacts-Klasse mit relativen Angaben.
(Habe mich vorher ein bisschen blöd ausgedrückt und statt FritzBox Callmonitor geschrieben)
In die Datenbank werden jedoch nur absolute eingetragen, also z.B. +49 1234 123456.
Diese werden dann Verarbeitet, sodass bei passender Vorwahl diese auch weggelassen werden kann.
Dauerbaustelle hat geschrieben:Was mir grade noch auffällt: Auf der Projektwebsite steht Python3, der Code ist aber Python2 (zumindest die `print`-Syntax ist nicht Python3-kompatibel). Wenn du da wirklich Python3 verwendest, kannst du für das Auspacken der Parameter von der FritzBox die neue Unpack-Syntax nutzen (wie in nem früheren Post beschrieben).
Ja, ich weiß, war auch ursprünglich so.
Habe ich nur noch nicht angepasst, ich würde auch gerne Python3 verwenden, wenn es das DBUS-Modul dafür geben würde.

Habe nun einiges geändert.
Mir gefällt es soweit gut, wie gefällt es Dir/Euch?

Vielen Dank und Viele Grüße
Maxi
Antworten