@nyma3: Das kommt nicht am Compiler vorbei weil bei der Definition von `SENDER` das Gleichheitszeichen für die Zuweisung fehlt.
Dann sind bei fast allen ``print``-Anweisungen unnötige und irreführende Klammern. Entweder weg mit den Klammern, oder `print_function` aus `__future__` importieren, damit aus ``print`` tatsächlich eine Funktion wird.
Um Bedingungen bei ``if`` gehören ebenfalls keine Klammern.
Die Semikolons am Zeilenende sind auch kein idiomatisches Python.
Auf den Kommentar ``# Variables:`` folgen Konstantendefinitionen und keine Variablen. Die auf Modulebene sowieso nicht stehen sollten.
``# SCRIPT:`` ist auch kein sinnvoller Kommentar. Kommentare sollten dem Leser einen Mehrwert über den Code bieten. Faustregel: Ein Kommentar beschreibt nicht was der Code macht, denn das beschreibt ja schon der Code selbst, sondern warum er das so macht was er macht. Aber auch nur, wenn das nicht offensichtlich ist.
Namen werden in Python klein_mit_unterstrichen geschrieben. Ausnahmen Konstanten (KOMPLETT_GROSS) und Klassen (MixedCase). Also `check_email()` statt `checkEmail()`.
Das Hauptprogramm steckt üblicherweise in einer Funktion die `main()` heisst. Also könnte man `loop()` umbenennen. Und dann auch gleich davor schützen das die Funktion aufgerufen wird wenn das Modul importiert statt als Programm ausgeführt wird.
Die rekursiven Aufrufe machen keinen Sinn. Weder in `check_email()` noch in `loop()`. Zumal, selbst wenn sie sinnvoll wären, währen sie in Python das falsche Mittel einfache Schleifen zu ersetzen. Python ist keine Programmiersprache in der „tail call optimization“ garantiert wird, und damit werden solche ”Schleifen” früher oder später zu einem Programmabruch mit einer Ausnahme führen weil der Aufrufstapel voll bzw. das Rekursionslimit erreicht ist.
Mit ``continue`` sollte man sparsam umgehen. Man kann den Programmfluss fast immer ohne dieses Schlüsselwort beschreiben. Im vorliegenden Code kann man beide Vorkommen auch einfach weglassen, ohne das sich etwas ändert.
`count` könnte man besser bennenen. Der Leser möchte ja gerne wissen *was* da gezählt wird. Die Behandlung dieses Zählers ist auch unnötig kompliziert. Man braucht den im ``else``-Fall nicht auf 10 zu setzen, denn den Wert hat der zu dem Zeitpunkt bereits. Man braucht aber auch gar nicht noch mal auf < 9 zu testen sondern kann den einfach bedingungslos um 1 erhöhen. Und den anderen Code aus dem ``else`` einfach *hinter* die ``while``-Schleife schreiben, denn die wird ja nur dann verlassen wenn der 10. Fehler aufgetreten ist. Da muss man nichts mehr extra prüfen. Das da „no connection“ ausgegeben wird ist falsch, denn es können auch andere Fehler zu dieser Ausgabe führen.
Das Schlafen von `CHECK_FREQ`-Sekunden kann man auch *einmal* im Hauptprogramm machen, statt es sowohl in den ``try``- als auch in den ``except``-Block zu schreiben. Im ``try``-Block steht es auch an einer eigenartigen Stelle. Warum will man zwischen dem Abfragen und Auswerten der Mail warten? Der Name ist falsch, denn es handelt sich gar nicht um eine Frequenz.
In der `check_email()`-Funktion macht das warten dann so überhaupt gar keinen Sinn. Warum‽
Da man den ``try``-Block so kurz wie möglich halten sollte, damit man wirklich nur Ausnahmen aus dem Code behandelt für die die Behandlung Sinn macht, sollte die Auswertung in einen ``else``-Block zu dem ``try`` wandern.
Wenn Du `email_message` aus `check_email()` ausserhalb der Funktion brauchst, dann musst Du den Wert an den Aufrufer zurückgeben. Sonst existiert der Wert nur lokal in der Funktion und nur solange der Funktionsaufruf läuft.
Wobei sich hier die Frage stellt *welchen* Wert Du da eigentlich zurückgeben möchtest, denn `email_message` bekommt in `check_email()` ja in einer Schleife potentiell mehr als einen Wert zugewiesen. Oder auch gar keinen, wenn die Suche keine Message-IDs liefert.
`mail` in `check_email()` hiesse besser `server` oder `mail_server`, denn es handelt sich ja nicht um eine Mail. Der Code meldet sich am Server an, aber nicht wieder ab. Das ist unsauber und es gibt Mailserver die einem das auf Dauer übel nehmen können. `IMAP4_SSL`-Objekte sind Kontextmanager, können also mit der ``with``-Anweisung verwendet werden.
Der `list()`-Aufruf beim Server macht keinen Sinn, weil Du mit dem Ergebnis überhaupt nichts machst.
`count` und `result` werden in der Funktion nie verwendet. `result` sollte vielleicht verwendet werden.
Statt die Konstante `SENDER` zu verwenden, steht der Wert noch mal im Quelltext.
Die ``for``-Schleife ist „unpythonisch“. Man kann in Python direkt über die Elemente von Seqzenztypen iterieren, ohne den Umweg über einen Index. Zudem ist das auch sehr ineffizient in jedem Schleifendurchlauf die Zeichenkette wieder erneut an Leerzeichen aufzuteilen, nur um *einen* Wert davon zu verarbeiten. Die Namen `i` und `x` die man sich hier sparen kann, sind zudem noch schlecht gewählt, da sehr nichtssagend. Wenn man zusätzlich zu den Elementen noch eine laufende Zahl benötigt, gibt es die `enumerate()`-Funktion.
Das `latest_` bei `latest_email_uid` macht keinen Sinn.
Das Dekodieren der kompletten RFC822-Nachricht als Text ist meiner Meinung nach falsch, denn die kann ja aus mehreren MIME-Teilen bestehen von denen jeder anders kodiert sein kann, oder sogar Teile enthalten die gar kein Text sondern beliebige Binärdaten sind. Ich würde an der Stelle die `imapclient`-Bibliothek empfehlen, die nicht so „low level“ wie `imaplib` ist.
Das ``if date_tuple:`` sorgt dafür das `local_message_date` nicht oder falsch definiert ist wenn die Bedingung nicht zutrifft.
`local_message_date` wird sehr komisch erstellt. `strftime()` liefert eine Zeichenkette. Die wandelst Du mit `str` in eine Zeichenkette um. Und die wird dann mit ``'%s' % …`` noch mal zur selben Zeichenkette formatiert. Was soll der Unsinn? Bei `strftime()` muss man bei Python 2 übrigens aufpassen, das dort Umlaute, zum Beispiel in Monatsnamen wie `März` ”irgendwie” kodiert sind in der Bytezeichenkette. Wie, hängt von den Systemeinstellungen ab.
Was soll diese `decode_header()`/`make_header()`-Orgie bewirken?
Den Index der UID für den Dateinamen zu verwenden ist kaputt. Der fängt ja jedes mal bei 0 an, das heisst das Programm wird die unterschiedlichsten E-Mail-Texte an die gleiche Datei anhängen. Besser man nimmt die UID selbst. (Sofern garantiert ist, das die vom IMAP-Server nicht wiederverwendet werden dürfen, das weiss ich im Moment nicht.)
Zeichenketten und Werte mit ``+`` und `str()` zusammenstückeln ist eher BASIC als Python. In Python gibt es dafür Zeichenkettenformatierung.
Dateien sollte man mit der ``with``-Anweisung verwenden um das schliessen auch in Ausnahmefällen zu garantieren.
Das Dateiformat ist komisch. Da wird für jeden Plaintext-MIME-Teil immer wieder Empfänger, Sender, Datum, und Betreff geschrieben. Das der Textteil dekodiert wird, ist ebenfalls falsch, denn a) muss der nicht UTF-8 kodiert sein und b) fällt das schreiben in die Datei auf die Nase wenn da überhaupt etwas anderes als ASCII enthalten ist. Denn dann müsste man die Unicode-Zeichenkette in Python 2 *selbst* explizit kodieren. Ich würde ja einfach die komplette RFC822-Nachricht speichern und gut ist. Die lässt sich dann später ganz normal mit verschiedensten Werkzeugen die mit E-Mails umgehen können, weiterverarbeiten.
Für jeden gespeicherten E-Mail-Teil wieder und wieder auszugeben das man sich jetzt beim `HOST` anmeldet ist unsinnig/falsch.
Ich komme dann ungefähr hierbei als Zwischenstand heraus:
Code: Alles auswählen
#!/usr/bin/python
from __future__ import absolute_import, division, print_function
import datetime
import email
import imaplib
import time
SENDER = 'BERECHTIGTE_EMAIL@GMAIL.COM'
EMAIL_ACCOUNT = 'RASPBERRY_EMAIL@gmail.com'
PASSWORD = 'RASPBERRY_PASSWORT'
HOST = 'imap.gmail.com'
MAILBOX = 'inbox'
EXPECTED_SUBJECT = 'CORRECT'
CHECK_DELAY = 3 # in seconds.
MAX_ERROR_COUNT = 10
def check_email():
with imaplib.IMAP4_SSL(HOST) as server:
print('Logging into', HOST)
server.login(EMAIL_ACCOUNT, PASSWORD)
server.select(MAILBOX)
_, data = server.uid(
'search', None, '(UNSEEN FROM "{0}")'.format(SENDER)
)
for uid in data[0].split():
_, data = server.uid('fetch', uid, '(RFC822)')
#
# FIXME Decoding seems to be dangerous as the complete
# RFC822 message could contain message parts in different
# encodings or even binary data that's not supposed to be
# decoded as text at all.
#
email_message = email.message_from_string(
data[0][1].decode('utf-8')
)
date_tuple = email.utils.parsedate_tz(email_message['Date'])
if date_tuple:
local_message_date = (
datetime.datetime
.fromtimestamp(email.utils.mktime_tz(date_tuple))
.strftime('%a, %d %b %Y %H:%M:%S')
)
else:
local_message_date = '-/-'
for part in email_message.walk():
if part.get_content_type() == 'text/plain':
file_name = 'email_{}.txt'.format(uid)
with open(file_name, 'a') as output_file:
#
# FIXME Writing a unicode string without
# encoding it will blow up if there is
# anything outside ASCII in it.
#
output_file.write(
u'From: {}\n'
u'To: {}\n'
u'Date: {}\n'
u'Subject: {}\n\n'
u'Body:\n\n{}'.format(
email_message['From'],
email_message['To'],
local_message_date,
email_message['Subject'],
#
# FIXME Message payload can be any
# other encoding. Hard coding UTF-8
# is wrong.
#
part.get_payload(decode=True).decode('utf-8'),
)
)
def main():
error_count = 0
while error_count < MAX_ERROR_COUNT:
print('establishing connection to ...', HOST)
try:
#
# FIXME `check_email()` currently doesn't return anything
# and also processes potentially no message at all or
# even more than one e-mail message.
#
email_message = check_email()
except IndexError:
error_count += 1
else:
if email_message['Subject'] == EXPECTED_SUBJECT:
print(
'- - - S U B J E C T C O R R E C T,'
' (message accepted) - - -'
)
else:
print(
'- - - S U B J E C T W R O N G,'
' (message declined) - - -'
)
time.sleep(CHECK_DELAY)
print('Something went wrong!', MAX_ERROR_COUNT, 'times.')
if __name__ == '__main__':
main()
@Sirius: Der `IndexError` kommt bei ``data[0]`` in `checkEmail()` wenn der IMAP-Server ein leeres Ergebnis liefert, weil beispielsweise keine ungesehenen Mails von dem Absender gefunden werden konnten. Schön/leicht ersichtlich ist dieser ”Test” allerdings wirklich nicht.