Foreach und Switch Case

Hallo

Ich bin gerade dabei eine Ampelsteuerung für mein Raspberry PI zu programmieren. Jetz bin ich auf ein Problem gestoßen, das ich irgendwie nicht verstehe. Von meinen Timer wird folgendes aufgerufen:

private void ElapsedEvent(object send, System.Timers.ElapsedEventArgs args)
        {
            var keys = currentPhase.Keys;
            Console.WriteLine($"Key count: {keys.Count}");
            foreach (var key in keys)
            {
                Console.WriteLine($"Current Key: {key}");
                var phase = currentPhase[key];
                var controller = trafficLightControllers[key];
                Console.WriteLine($"Phase: {phase}");
                Console.WriteLine($"Controller: {controller.ControllerGuid}");
                switch (phase)
                {
                    case TrafficPhase.RedPhase:
                        currentPhase[key] = controller.SwitchPhase(TrafficPhase.RedYellowPhase);
                        MachineStateEventHandler?.Invoke(this, new MachineStateEventArgs("Activate redsignal and yellowsignal", currentPhase[key], controller.ControllerGuid));
                        timer.Interval = 2000;
                        break;
                    case TrafficPhase.RedYellowPhase:
                        currentPhase[key] = controller.SwitchPhase(TrafficPhase.GreenPhase);
                        MachineStateEventHandler?.Invoke(this, new MachineStateEventArgs("Activate greensignal", currentPhase[key], controller.ControllerGuid));
                        timer.Interval = 10000;
                        break;
                    case TrafficPhase.YellowPhase:
                        currentPhase[key] = controller.SwitchPhase(TrafficPhase.RedPhase);
                        MachineStateEventHandler?.Invoke(this, new MachineStateEventArgs("Activate redsignal", currentPhase[key], controller.ControllerGuid));
                        timer.Interval = 5000;
                        break;
                    case TrafficPhase.GreenPhase:
                        currentPhase[key] = controller.SwitchPhase(TrafficPhase.YellowPhase);
                        MachineStateEventHandler?.Invoke(this, new MachineStateEventArgs("Activate yellowsignal", currentPhase[key], controller.ControllerGuid));
                        timer.Interval = 2000;
                        break;
                    default:
                        Console.WriteLine($"Undefined: {key} or {phase}");
                        break;
                }
            }
        }

Das Problem ist jetzt das der Switch Case dafür sorgt das die Foreach Schleife nicht richtig durchlaufen wird. Es wird immer nur das erste Element durchlaufen.
Der Sowohl currentPhase als auch trafficlightControllers sind Dictionarys mit dem selben Key Typ.
Nehme ich den Switch case raus, funktioniert meine Foreachschleife Problemlos. Im www konnte dazu nix finden. Ich wollte einfach wissen ob das verhalten richtig mit dem Switch Case und der Foreach Loop richtig ist.

Wofür hast du überhaupt die foreach-Schleife? Es soll doch pro Durchlauf des Timer-Ereignisses nur genau eine Phase weitergehen.

Hast du den Code mal debugged und geschaut ob dieser generell so funktioniert wie er soll?
Ansonsten sieht das switch/case erst einmal in Ordnung aus und erklärt für mich nicht, warum er nur einen Durchlauf machen sollte.

Ich würde den ganzen Schleifen Körper auch in eine eigene Methode packen.
Das macht den Code enorm les- und wartbarer.

T-Virus

Hallo,

Ja das ist prinzipel richtig. Die foreach schleife habe ich, weil die Ampelanlage 4 Ampeln steuern soll, diese habe ich aktuell in einem Dictionary drin. Um die Phase zu wechseln muss ich einmal alle Ampeln aufrufen.

Ich habe im groben folgende Klassen:* Klasse für ein Lichtsignal mit den Properties für die Farbe, Status und Pin

• Klasse für eine Ampel mit 3 Lichtsignale für Rot, Gelb und Grün
• Controller Klasse für eine Ampel
• Klasse welche die Gesamtanlage steuert und die Controller Klassen aktuell in einem Dictionary hält

Mein geposteter Code ausschnitt kommt aus der letzten Klasse.

@T-Virus:

Ja Laut Konsolen ausgabe scheint er nur einmal durch die Schleife zu gehen.
Log mit aktiven Switch Case:

Key count: 4
Current Key: North
Phase: RedPhase
Controller: ef0d21a7-c71d-4f72-9a1d-787afbc523f9
Controller: ef0d21a7-c71d-4f72-9a1d-787afbc523f9 send message: Activate redsignal and yellowsignal and has phase: RedYellowPhase
Key count: 4
Current Key: North
Phase: RedYellowPhase
Controller: ef0d21a7-c71d-4f72-9a1d-787afbc523f9
Controller: ef0d21a7-c71d-4f72-9a1d-787afbc523f9 send message: Activate greensignal and has phase: GreenPhase

Log ohne Switch Case:

Key count: 4
Current Key: North
Phase: RedPhase
Controller: 6d48cfb3-32e6-4e91-84a8-9e0419201c01
Current Key: East
Phase: RedPhase
Controller: 59b02032-bcca-44d2-990f-07b1dfd99476
Current Key: South
Phase: RedPhase
Controller: b7736796-010f-437d-9ebf-482e73622941
Current Key: West
Phase: RedPhase
Controller: 83581af1-15b4-4e2f-b81c-d00b9208ce46
Key count: 4
Current Key: North

Das läuft Übrigens auf dem Raspberry.

Du hast keinerlei Fehlerhandling und loggst bei einer Exception auch nichts.

Ein dicker Patzer ist zB, dass Du die aktuelle Liste änderst, während sie durchlaufen wird.
Und war mit folgender Zeile:

 currentPhase[key] = controller.SwitchPhase(TrafficPhase.GreenPhase);

Das müsste eigentlich eine Exception geben.
Die wirst Du aber wegen fehlendem Logging, und weil es hier ein Event ist, wohl nie mitbekommen.

😭

Danke Abt, das war der entscheidene Hinweis. Ich hab mein Code um eine try/catch blog erweitert und es kam der erwartete Fehler im log.

Danke euch für eure Hilfe.

Trotzdem ist da noch ein Logikfehler, da du innerhalb der Ereignismethode ja mittels timer.Interval = ... das Intervall änderst, dieses dann aber für alle 4 Ampeln gleichzeitig gilt.
Du mußt je Ampel einen eigenen Timer verwenden (bzw. einen Timer mit der höchsten Frequenz erzeugen und je Phase einen Zähler (als Faktor) verwenden).

Hallo,

Ich habe die Methode inzwischen etwas umgeschrieben. Die gegenüberliegenden Ampeln sollten synchron miteinander laufen. Die Methode GetCurrentComtrolles() gibt die jeweils gegenüberliegenden Ampeln zurück je nachdem wie der wert von isNorthSouth ist. Dadurch werden jetzt bei jedem Event nur 2 Ampeln geschaltet außerdem habe ich nun einen kurzen Augenblick wo alle Ampeln Rot sind.

Eigentlich ist das Thema abgeschlossen, weil mein eigentliches Problem gelöst ist.

 private void ElapsedEvent(object send, System.Timers.ElapsedEventArgs args)
        {
            try
            {
                var controllers = GetCurrentControllers();
                foreach (var item in controllers.Select((v, i) => new { controller = v, index = i }))
                {
                    switch (item.controller.CurrentPhase)
                    {
                        case TrafficPhase.RedPhase:
                            item.controller.SwitchPhase(TrafficPhase.RedYellowPhase);
                            FireEvent(item.controller, "Activate redsignal and yellowsignal");
                            timer.Interval = 2000;
                            break;
                        case TrafficPhase.RedYellowPhase:
                            item.controller.SwitchPhase(TrafficPhase.GreenPhase);
                            FireEvent(item.controller, "Activate greensignal");
                            timer.Interval = 10000;
                            break;
                        case TrafficPhase.YellowPhase:
                            item.controller.SwitchPhase(TrafficPhase.RedPhase);
                            FireEvent(item.controller, "Activate redsignal");
                            if(item.index == 0) 
                            {
                                isNorthSouth = !isNorthSouth;
                            }
                            timer.Interval = 1000;
                            break;
                        case TrafficPhase.GreenPhase:
                            item.controller.SwitchPhase(TrafficPhase.YellowPhase);
                            FireEvent(item.controller, "Activate yellowsignal");
                            timer.Interval = 2000;
                            break;
                        default:
                            break;
                    }
                }

            }
            catch (Exception ex)
            {
                Console.WriteLine($"Exception: {ex}");
                throw;
            }
        }

... das wäre eine schöne Aufgabe für das state pattern.