Wie markbee schon bemerkt hat, wird bei "if (valLDR > 800)" nur die Zeile ausgeführt die "connected" ausgibt - eine Mail sollte dagegen immer versendet werden. Ist das so?
Ansonsten hier noch ein kleiner Crashkurs in Clean-Code-Developement, der das Debugging vieleicht erleichtert
(Mangels Arduino + Ethernetshield hier vor Ort leider ungetestet)
1.) Refactoring... Separation of Concerns + Single Responsibility Principle
Codeabschnitte in einzelne Funktionen auslagern, jede Funktion sollte dabei genau eine Aufgabe übernehmen. Im konkreten Fall gehört das Mailabholen, Mailsenden und die Prüfung der Eingaben jeweils in eine eigene Funktion mit aussagekräftigem Namen.
Vorteile: Eine Funktion mit einer Aufgabe ist leichter zu verstehen, finden oder nachträglich zu bearbeiten. Man hat abgeschlossene kleine Abschnitte die man einzeln (in der professionellen Softwareentwicklung dann auch automatisiert) testen kann. Wenn sie für sich alleine erstmal funktionieren kann man sie auch ausblenden + vergessen (also wie ich erst kürzlich hier im Forum gelernt habe in der Arduino-IDE in Tabs auslagern), womit sie den Lesefluss nicht mehr stören. Weiterhin hat man eine Abstraktionsebene eingeführt, bei der die konkrete Implementierung leicht austauschbar ist - die Funktion "boolean isLight()" kann man mit einem LDR, einer Fotodiode oder Kamera lösen ohne die Logik der gesamten Applikation zu beeinträchtigen; die Funktion "sendMail()" könnte man auch mit der PHP-Methode ersetzen.
In die mail-Funktionen gehört dann auch das Einlesen der Antwort vom Mailserver, und nicht wie in allen gängigen Beispielen erst im Loop was ja prinzipbedingt nur einmal während der gesamten Laufzeit des Sketches funktionieren würde, da ja nur einmal eine einzige Clientverbindung im setup() gemacht wird. Überhaupt ist das ein sehr fragwürdiges Vorgehen, die Verbindung im setup() zu öffnen und im loop() die Antwort auszulesen. Das könnte man bei einem Chatclient, wo die Verbindung einmal geöffnet wird und dann weiterhin immer offen bleibt so machen, aber hier ergibt das nicht viel Sinn. Vieleicht steckt da auch irgendwo der Fehler, wenn im setup() die Bedingung nicht erfüllt ist, passiert auch später einfach nichts mehr.
boolean sendMail() {
if (client.connect()) {
client.println("HELO HalloServer");
client.println("MAIL FROM: hiersteht@meineemail.de");
client.println("RCPT TO: hierdie@empfängermail.de");
client.println("DATA");
client.println("TO: hierdie@empfängermail.de");
client.println("SUBJECT: Arduino sendet Email");
client.println();
client.println("Das Licht ist aus.");
client.println(".");
client.println("QUIT");
while (client.available()) {
char c = client.read();
Serial.print(c);
}
client.stop();
return true;
} else {
return false;
}
}
boolean getMail() {
if (popclient.connect()) {
popclient.println("USER sui@gmx.de");
popclient.println("PASS passwort");
popclient.println("QUIT");
while (client.available()) {
char c = client.read();
Serial.print(c);
}
popclient.stop();
return true;
} else {
return false;
}
}
boolean isLight() {
int valLDR = analogRead(LDR);
return (valLDR > 800);
}
2.) Testen
Die Funktionen lassen sich jetzt komfortabel einzeln testen:
void setup() {
Ethernet.begin(mac, ip);
Serial.begin(9600);
delay(1000);
if (getMail()) {
Serial.println("Mail abholen erfolgreich");
} else {
Serial.println("Mail abholen fehlgeschlagen");
}
if (sendMail()) {
Serial.println("Mail senden erfolgreich");
} else {
Serial.println("Mail senden fehlgeschlagen");
}
}
void loop() {
if (isLight()) {
Serial.println("Es ist hell");
} else {
Serial.println("Es ist dunkel");
}
delay(1000);
}
3.) Zusammenbauen
So, die eigentliche Applikationslogik besteht jetzt - und hier zeigt sich sehr schön der Vorteil von sauberem Code - aus einem wirklich übersichtlichen, leicht lesbaren und selbsterklärenden 4-Zeiler. Sollte da tatsächlich noch ein Fehler drin sein, lässt der sich sicher einfach finden
void loop() {
if (isLight()) {
getMail();
sendMail();
}
}
4. Noch mehr Refactoring
Eigentlich sind wir hier ja schon fertig, es sei denn das ganze soll noch weiter aufgeräumt werden...
sendMail() müsste eigentlich aufgegliedert werden in sendMail(), sendMailSMTP(), sendMailSMTPafterPOP() weil das getMail() nicht zur Applikationslogik gehört und aus dem loop() rausfliegen müsste. Vieleicht verwendest du aber auch einen Mailserver ohne/anderer Authentifizierung oder einen Webmailer, dann entsprechende sendMailBasicAuth() oder sendMailPHPWebmailer() oder so implementieren und in sendMail() aufrufen.
boolean sendMail() {
return sendMailSMTPafterPOP();
}
boolean sendMailSMTPafterPOP() {
if (!getMail()) {
return false;
}
return sendMailSMTP();
}
boolean sendMailSMTP() {
// so wie vorher sendMail()
}
-
Aus sendMail() wird dann sicher noch ein "Don't repeat yourself"-wiederverwendbares sendMail(string To, string Subject, string Body).
-
Die Mailzugangsdaten gehören eigentlich am Anfang in Konstanten definiert, genauso wie der Schwellenwert 800 für den LDR.
-
Für ein besseres Verständniss würde ich die Variablen server + client in SMTPServer + SMTPClient umbenennen, analog dazu POPServer + POPClient.
-
Fehlerbehandlung: Die Antworten der Mailserver könnte man in den Mailfunktionen auch verarbeiten.