Das deutsche Python-Forum

Hallo,

mein kleines Programm läuft zwar, aber ich würde nun gerne den Code optimieren und übersichtlicher machen.

Ich habe folgende Hardware die ich über den Raspi steure:
Lichter
Motoren
Temperaturen
Luftfeuchte
Helligkeit

Ich würden gerne im Code das Licht so ansprechen:
Light.led1.manual.on oder Light.led1.manual = Light.on
Light.led1.manual.off
Light.led1.manual.toggle
...

Dazu habe ich folgendes gefunden:

Code: Alles auswählen

Class P:
    def __init__(self,x):
        self.__x = x

    def __getX(self):
        return self.__x

    def __setX(self, x):
        self.__x = x

    x = property(__getX, __setX)

Also Getter/Setter

Code: Alles auswählen

>>> from p import P
>>> a = P(19)
>>> b = P(10)
>>> c = a.x + b.x
>>> c
29

Meine Licht Klasse sollte folgende Objekte haben:
led1
led2
led3
led4
led5
led6
led_outdoor

jedes der Lichter hat einen eigenen Wert für:
manual
auto
override

Wenn ich den Modus von "Manual" auf "auto" schalte, aktualisieren die Ausgänge nach den Werten von Auto, usw

Beispiel:
Light.mode.auto
oder
Light.mode = Light.auto

Wie baut man grob so eine Klasse?

Herzlichen Dank!

@unique24: das was Du da gefunden hast, ist schlechter Code. Doppelte Unterstriche benutzt man nicht. Und für property benutzt man Dekoratoren. Der Klassenname ist nichtsagend und get und set sind so trivial, dass man keine Properties braucht.
Aber bevor Du mit Dekoratoren anfängst, solltest Du erst einmal die Funktionalität mit normalen Methoden programmieren. Also ein set_mode. Was soll denn bei "auto" passieren?

@unique24: Statt der doppelten führenden Unterstriche würde man da nur *einen* machen und Namen schreiben die sich auch sonst an die Konventionen in Python halten, also klein_mit_unterstrichen für Methoden und Attribute.

Und das Property hätte man auch in Python 2.7 schon nicht mehr so geschrieben, denn Properties waren einer *der* Anwendungsfälle für die Decorator-Syntax.

Code: Alles auswählen

class Example:
    def __init__(self, x):
        self._x = x

    @property
    def x(self):
        return self._x
    
    @x.setter
    def x(self, value):
        self._x = value

Allerdings ist dieses Beispiel zwar nett um die Syntax zu zeigen, aber inhaltlich ist das totaler Quatsch weil man bei trivialen Gettern und Settern das ganze Geraffel einfach weglassen kann, ohne das sich für den Benutzer der Klasse irgendetwas ändert. Python ist nicht Java. Gleichwertige Klasse:

Code: Alles auswählen

class Example:
    def __init__(self, x):
        self.x = x

Die Klasse `Light` sollte wohl `Lights` heissen oder `LEDs`, denn sie repräsentiert ja nicht nur ein Licht/eine LED sondern mehrere.

Die Namen der Attribute sind komisch und fühlen sich falsch an. Wenn das variable Namen sind, dann sollten das keine Namen sondern Schlüssel in einem Wörterbuch sein. Oder zumindest eine Liste mit den nummerierten LEDs statt Namen die nummeriert sind. Das ist immer ein Warnzeichen, das man etwas komisches bis falsches macht.

Den Modus kann man mit einem `enum.Enum`-Datentyp modellieren.

Ansonsten würde ich mal sagen Du solltest da nicht von irgendwelcher magischen Syntax ausgehen die Du gerne hättest, sondern das erst einmal ganz normal entwerfen. Und da würde man bei der Implementierung eher mit dem anfangen was Du hier gerade nicht zeigst, nämlich den einzelnen LED-Objekten.

Was verwendest Du da denn? Weil `gpiozero` ja beispielsweise schon Klassen für so etwas hat.

Hallo,

ok, hab mal einen Ansatz geschrieben:
main.py:

Code: Alles auswählen

from lights import Lights


class Demo():

    lights = Lights()

    def __init__(self):
        self.demo_function()

    def demo_function(self):
        print(self.lights.led1.get_status())


if __name__ == '__main__':
    Demo()

lights.py:

Code: Alles auswählen

from light import Light


class Lights:

    def __init__(self):
        self.led1 = Light("LED 1")
        self.led2 = Light("LED 2")
        self.led2 = Light("LED 3")

light.py:

Code: Alles auswählen

from enum import Enum


class Switch(Enum):
    off = 0
    on = 1
    undef = -1


class Mode(Enum):
    manual = 0
    auto = 1
    override = -1


class Light:

    def __init__(self, name):
        self.name = name
        self.value = Switch.undef

    def get_status(self):
        return self.value

Es gibt die Klasse "Licht" ... die Klasse "Lichter" definiert alle zur Verfügung stehenden Lichter.
Die main ruft einen Beispiel auf.

In wie weit ist das mal ok?

Nochmal Hallo,

hab die Klasse "Light" nun mal erweitert.
Jedes Licht hat 3 mögliche Zustände:
manual
automatic
override

Jenachdem in welchem Modus das Licht arbeitet, wir der Status und Zustand aktualisiert:

Code: Alles auswählen

from lights import Lights
from light import Switch
from light import Mode


class Demo():
    lights = Lights()

    def __init__(self):
        self.demo_function()

    def demo_function(self):
        print("LED 1: " + self.lights.led1.get_status().name)
        self.lights.led1.switch(Switch.on, Mode.manual)
        print("LED 1: " + self.lights.led1.get_status().name)
        self.lights.led1.switch(Switch.off, Mode.automatic)
        print("LED 1: " + self.lights.led1.get_status().name)
        self.lights.led1.set_mode(Mode.automatic)
        print("LED 1: " + self.lights.led1.get_status().name)


if __name__ == '__main__':
    Demo()

Code: Alles auswählen

from enum import Enum


class Switch(Enum):
    off = 0
    on = 1
    undef = -1


class Mode(Enum):
    manual = 0
    automatic = 1
    override = -1


class Light:

    def __init__(self, name):
        self.name = name

        self.value_manual = Switch.undef
        self.value_auto = Switch.undef
        self.value_override = Switch.undef

        self.current_value = Switch.undef
        self.current_state = Switch.undef
        self.current_mode = Mode.manual

    def get_status(self):
        return self.current_state

    def get_mode(self):
        return self.current_mode

    def switch(self, value, mode):
        if mode == Mode.manual:
            self.value_manual = value
            self.update_gpio()
        if mode == Mode.automatic:
            self.value_auto = value
            self.update_gpio()
        if mode == Mode.override:
            self.value_override = value
            self.update_gpio()

    def set_mode(self, new_mode):
        self.current_mode = new_mode
        if new_mode == Mode.manual:
            self.update_gpio()
        if new_mode == Mode.automatic:
            self.update_gpio()
        if new_mode == Mode.override:
            self.update_gpio()

    def update_gpio(self):
        # GPIO aktualisieren
        if self.current_mode == Mode.manual:
            self.current_value = self.value_manual
            self.current_state = self.current_value
        if self.current_mode == Mode.automatic:
            self.current_value = self.value_auto
            self.current_state = self.current_value
        if self.current_mode == Mode.override:
            self.current_value = self.value_override
            self.current_state = self.current_value

Ausgabe:
LED 1: undef
LED 1: on
LED 1: on
LED 1: off

Man schreibt nicht für jede Klasse ein eigenes Modul. Leights ist auch nicht wirklich eine sinnvolle Klasse, Das könnte genauso gut eine Liste oder ein Wörterbuch sein. Demo ist auch keine Klasse sondern nur eine kompliziert geschriebene Funktion, lights darf kein Klassenattribut sein.
Was ist der Unterschied zwischen current_value und current_state?
Benutze Wörterbücher, dann sparst Du dir die vielen if-Abfragen. Eine Fehlermeldung bei falschen Eingaben fehlt noch.

@unique24: Also diese Getter und Setter sahen ja schon so furchtbar „javaesque“ aus, und jetzt steckst Du auch noch jede Klasse in ein eigenes Modul. Das ist Python verdammt. Ein Modul ist dazu da um zusammengehörige Funktionen und Klassen zu bündeln. Wenn man jede Klasse in ein eigenes Modul steckt ist das so als würde man in Java jede Klasse noch mal einzeln in ein Package stecken. Das macht keinen Sinn.

Die `Demo`-Klasse verstärkt diesen Java-Eindruck auch noch mal, denn das ist ja überhaupt keine Klasse, sondern einfach nur Funktionen die sinnfrei in eine Klasse gesteckt wurden und die die globale Variable `Demo.lights` verwenden. Das `lights`-Objekt gehört nicht als Attribut auf die Klasse. Da gehören in aller Regel nur Konstanten hin.

Eine `__init__()` ist dazu da ein Objekt zu initialisieren so dass der Aufrufer das erstellte, initialisierte Objekt danach benutzten kann. Wenn die `__init__()` erst wieder zurück kehrt wenn im Grunde das gesamte Programm abgelaufen ist, dann stimmt was mit dem Entwurf nicht. Python ist nicht Java, man muss nicht alles zwanghaft in Klassen stopfen.

Zu der `Lights` hatte ich ja bereits gesagt das man keine nummerierten Namen verwendet. Das ist einfach eine Liste von `Light`-Objekten und es macht keinen Sinn die als solche noch mal in einem Objekt zu verpacken das sonst keinerlei Funktionalität hat.

Die `Enum`-Werte sind Konstanten und Konstanten werden per Konvention KOMPLETT_GROSS geschrieben. Die Werte der Werte sind auch komisch. Warum verwendest Du da die Zahlen 0, 1, und -1? Die Werte sind egal wenn man die nicht irgendwo tatsächlich verwendet. `Enum` ist kein Zahltyp wie in C oder C++!

Warum gibt `get_status()` etwas zurück was `*_state` heisst? Das sollte angeglichen werden.

Wenn es nicht auch einen nicht-aktuellen Wert für `current_state`, `current_value`, und `current_mode` gibt, dann ist das `current_*` überflüssig, denn das ist ja der Normalfall, dass Attribute den aktuellen Zustand eines Objekts bestimmen.

Bei `state` scheint mir der Getter überflüssig zu sein.

Bei den ``if``\s bezüglich dem Modus schliessen die sich ja gegenseitig aus, also ist ``elif`` angesagt. Beziehungsweise in `set_mode()` macht die Abfrage ja gar keinen Sinn, weil in jedem Fall das gleiche passiert.

Bei den `value_*`-Attributen ist die Reihenfolge so Yoda-mässig. Das ist ja nicht der ”Wert Automatik” sondern der ”Automatikwert”. Aber das sollte vielleicht sowieso eher ein Wörterbuch sein das Modus auf Schalterzustand abbildet. Dann fallen die ganzen ``if``\s bezüglich Modus in beiden Methoden zu jeweils nur einer einzigen Zeile zusammen.

Zwischenstand (in *einem* Modul):

Code: Alles auswählen

#!/usr/bin/env python3
from enum import auto as enum_value, Enum


class SwitchState(Enum):
    OFF = enum_value()
    ON = enum_value()
    UNDEFINED = enum_value()


class Mode(Enum):
    AUTOMATIC = enum_value()
    MANUAL = enum_value()
    OVERRIDE = enum_value()


class Light:
    def __init__(self, name):
        self.name = name
        self.mode_to_switch_state = dict.fromkeys(Mode, SwitchState.UNDEFINED)
        self.state = SwitchState.UNDEFINED
        self._mode = Mode.MANUAL

    @property
    def mode(self):
        return self._mode

    @mode.setter
    def mode(self, value):
        self._mode = value
        self.update_gpio()

    def switch(self, state, mode):
        self.mode_to_switch_state[mode] = state
        self.update_gpio()

    def update_gpio(self):
        self.state = self.mode_to_switch_state[self.mode]


def main():
    lights = [Light(f"LED {i + 1}") for i in range(3)]

    light = lights[0]
    print("LED 1:", light.state.name)
    light.switch(SwitchState.ON, Mode.MANUAL)
    print("LED 1:", light.state.name)
    light.switch(SwitchState.OFF, Mode.AUTOMATIC)
    print("LED 1:", light.state.name)
    light.mode = Mode.AUTOMATIC
    print("LED 1:", light.state.name)


if __name__ == "__main__":
    main()

Hallo,

da habe ich nun ja einiges zu tun

Einige Antworten:

Was ist der Unterschied zwischen current_value und current_state?

current_value ist der zu schaltende Wert
current_state ist der tastächliche Wert

current_value wird auf das GPIO angewendet und wenn ein Fehler erhält current_state den tatsächlichen Zustand.
Kann ich aber wohl in einem Wert machen

Die Demo Klasse habe ich schnell getippt, um die Klassen zu testen.

Warum verwendest Du da die Zahlen 0, 1, und -1?

Weil ich einen booleschen Wert schalte .. aber auch den Zustand Undefiniert haben möchte. Wenn also der Zustand eines GPIO nicht bekannt ist. Kann man auch False, True und -1 nutzen?

Die Lights Klasse gibt es, weil noch viele Funktionen eingebaut werden:
schalte alle Lichter
toggle alle Lichter
Sonnenaufgang Simulation
Sonnenuntergang Simulation
...

Herzlichen Dank für eure Hilfe. Und ja, "komme" aus der Java Ecke

Hallo,

hab das nun mal umgesetzt und etwas abgeändert/erweitert:

force: Durch den wechsel in den Modus "force" wird dieses Flag gesetzt ... der aktuelle Betriebsmodus (manual, automatic) soll gespeichert werden ... wenn force wieder AUS wird, soll der aktuelle Zustand wieder hergestellt werden. Den erzwungen Modus kann man nur durch eine Funktion wieder abschalten.

toggle: Ich möchte das Licht umschalten. Aber mit "not" geht das durch das enum nimmer, richtig?

state: Diese habe ich nur lesend gemacht ... es soll ja nie gesetzt werden sondern aus dem Ergebnis wenn ein GPIO geschalten wird

Ist das nun soweit korrekt?

Code: Alles auswählen

#!/usr/bin/env python3
from enum import auto as enum_value, Enum


class SwitchState(Enum):
    OFF = enum_value()
    ON = enum_value()
    UNDEFINED = enum_value()


class Mode(Enum):
    AUTOMATIC = enum_value()
    MANUAL = enum_value()
    FORCE = enum_value()


class Light:
    def __init__(self, name):
        self.name = name
        self.mode_to_switch_state = dict.fromkeys(Mode, SwitchState.UNDEFINED)
        self._state = SwitchState.UNDEFINED
        self._mode = Mode.MANUAL
        self._force = SwitchState.OFF
        self._last_mode = Mode.MANUAL

    @property
    def mode(self):
        return self._mode

    @mode.setter
    def mode(self, new_mode):
        if new_mode == Mode.FORCE:
            self._force = SwitchState.ON
            self._last_mode = self._mode
            print("saved", self._last_mode)
            self._mode = new_mode
        elif self._force == SwitchState.OFF:
            self._mode = new_mode
        self.update_gpio()

    def force_off(self):
        if self._force == SwitchState.ON:
            self._mode = self._last_mode
            self._force = SwitchState.OFF
            self.update_gpio()

    @property
    def state(self):
        return self._state

    def switch(self, state, mode):
        self.mode_to_switch_state[mode] = state
        self.update_gpio()

    def toggle(self, mode):
        if self.mode_to_switch_state[mode] == SwitchState.ON:
            self.mode_to_switch_state[mode] = SwitchState.OFF
        else:
            self.mode_to_switch_state[mode] = SwitchState.ON
        self.update_gpio()

    def update_gpio(self):
        if self._force == SwitchState.ON:
            self._state = self.mode_to_switch_state[Mode.FORCE]
        else:
            self._state = self.mode_to_switch_state[self.mode]

    def __str__(self):
        return self.name + ": " + str(self._state) + " in " + str(self.mode) + \
               "\n\t\t" + str(self.mode_to_switch_state)


def main():
    lights = [Light(f"LED {i + 1}") for i in range(3)]

    light = lights[0]
    print(light)

    print("switch ON in MANUAL")
    light.switch(SwitchState.ON, Mode.MANUAL)
    print(light)

    print("switch OFF in AUTOMATIC")
    light.switch(SwitchState.OFF, Mode.AUTOMATIC)
    print(light)

    print("switch to AUTOMATIC")
    light.mode = Mode.AUTOMATIC
    print(light)

    print("force ON")
    light.switch(SwitchState.ON, Mode.FORCE)
    light.mode = Mode.FORCE
    print(light)

    print("switch OFF in MANUAL")
    light.switch(SwitchState.OFF, Mode.MANUAL)
    print(light)

    print("force OFF")
    light.force_off()
    print(light)


if __name__ == "__main__":
    main()

@unique24: Die magischen Methoden werden üblicherweise als erstes definiert. Weniger strikt, aber doch recht verbreitet ist die Konvention das dann die Properties vor den Methoden kommen.

Zusammenstückeln von Werten und Zeichenketten per `str()` und ``+`` ist eher BASIC denn Python. Python hat dafür Zeichenkettenformatierung mit der `format()`-Methode und f-Zeichenkettenliterale.

Sehe ich das richtig das `_force` nur Wahr oder Unwahr sein kann? Dann würde ich da keine dreiwertigen `Enum`-Typ für verwenden, sondern einfach `bool`, oder einen neuen `Enum`-Typ der nur für `_force` verwendet wird. Das ginge aber IMHO schon sehr Richtung overengineering.

Wobei ich denke wir haben da gerade wieder ein Problem den Zustand nicht-redundant auszudrücken. Oder kann es sein das ``_force == True`` gilt aber gleichzeitig der `mode` *nicht* `Mode.FORCE` ist? Damit is `_force` überflüssig. Mach doch nicht Sachen komplizierter als sie sein müssten.

Das Verhalten wenn man zweimal hintereinander `Mode.FORCE` an `mode` zuweist ist wohl eher falsch‽ Zeit für den Hinweis auf Unittests und hier auch wieder: Python ist nicht Java. Schau Dir das externe `pytest` an, bevor Du anfängst die hässliche XUnit-API aus der Standardbibliothek zu verwenden.

``not`` geht mit der `Enum` nicht mehr, aber es ist ja auch eine dreiwertige Logig, da ginge ``not`` auch nicht wirklich wenn das drei andere Werte wären.

Ich würde die Logik in den `Enum`-Typ packen und so wie ich das sehe ist die im Moment auch falsch oder für meinen Geschmack doch sehr überraschend. Beim `toggle()` ON → OFF und OFF → ON ist klar, aber bei UNDEFINED hätte ich ja wieder UNDEFINED erwartet und nicht ON‽

Zwischenstand (ungetestet):

Code: Alles auswählen

#!/usr/bin/env python3
from enum import auto as enum_value, Enum


class SwitchState(Enum):
    OFF = enum_value()
    ON = enum_value()
    UNDEFINED = enum_value()

    def toggled(self):
        return self._TOGGLED[self]


SwitchState._TOGGLED = {
    SwitchState.OFF: SwitchState.ON,
    SwitchState.ON: SwitchState.OFF,
    SwitchState.UNDEFINED: SwitchState.UNDEFINED,
}


class Mode(Enum):
    AUTOMATIC = enum_value()
    MANUAL = enum_value()
    FORCE = enum_value()


class Light:
    def __init__(self, name):
        self.name = name
        self.mode_to_switch_state = dict.fromkeys(Mode, SwitchState.UNDEFINED)
        self._state = SwitchState.UNDEFINED
        self._mode = Mode.MANUAL
        self._previous_mode = Mode.MANUAL

    def __str__(self):
        return (
            f"{self.name}: {self.state} in {self.mode}"
            f"\n\t\t{self.mode_to_switch_state}"
        )

    @property
    def mode(self):
        return self._mode

    @mode.setter
    def mode(self, value):
        if self.mode is not Mode.FORCE:
            if value is Mode.FORCE:
                self._previous_mode = self._mode
                print("saved", self._previous_mode)
            self._mode = value
            self.update_gpio()

    @property
    def state(self):
        return self._state

    def force_off(self):
        if self.mode is Mode.FORCE:
            self._mode = self._previous_mode
            self.update_gpio()

    def switch(self, state, mode):
        self.mode_to_switch_state[mode] = state
        self.update_gpio()

    def toggle(self, mode):
        new_state = self.mode_to_switch_state[mode]
        self.mode_to_switch_state[mode] = new_state.toggled()
        self.update_gpio()

    def update_gpio(self):
        self._state = self.mode_to_switch_state[self.mode]


def main():
    lights = [Light(f"LED {i + 1}") for i in range(3)]

    light = lights[0]
    print(light)

    print("switch ON in MANUAL")
    light.switch(SwitchState.ON, Mode.MANUAL)
    print(light)

    print("switch OFF in AUTOMATIC")
    light.switch(SwitchState.OFF, Mode.AUTOMATIC)
    print(light)

    print("switch to AUTOMATIC")
    light.mode = Mode.AUTOMATIC
    print(light)

    print("force ON")
    light.switch(SwitchState.ON, Mode.FORCE)
    light.mode = Mode.FORCE
    print(light)

    print("switch OFF in MANUAL")
    light.switch(SwitchState.OFF, Mode.MANUAL)
    print(light)

    print("force OFF")
    light.force_off()
    print(light)


if __name__ == "__main__":
    main()

Die Asymmetrie das man `Mode.FORCE` zuweisen kann, das dann aber nur über `force_off()` wieder los wird, finde ich ja irgendwie unschön. Vielleicht wäre da ein `force` doch nicht schlecht und stattdessen keinen `Mode.FORCE`. Statt `force` wäre dann `mode_locked` vielleicht auch ein verständlicherer Name dafür. Und dafür bräuchte man dann kein `_previous_mode` mehr. Vielleicht habe ich aber auch nicht verstanden was das alles soll — ich finde das insgesamt ein bisschen unübersichtlich und sehe nicht so ganz was da überhaupt erreicht werden soll.

Hallo,

vielen Dank ... werd ich genauer studieren.

Zum Zweck des ganzen:
Die Steuerung die ich geschrieben habe ist für einen Hühnerstall. Es läuft im Auto Modus und kann auch per Web gesteuert werden.
Das Tür geht bei Dämmerung auf/zu, es wird die Temperatur außen/innen erfasst, sowie Helligkeit und Luftfeuchte.
Eine cam hängt auch noch am Raspi

Das Licht als Beispiel hier:
Es gibt 6 LED Strips an der Decke. Die dienen natürlich zum ein/aus schalten wenn man rein geht, aber es wird auch ein Sonnen Aufgang und Sonnen Untergang simuliert sowie eine Mind. Beleuchtungsdauer im Stall.
Es gibt auch eine Außenleuchte, ohne spezielle Logik

Im Auto Modus:
Um 6:00 startet der Sonnenaufgang wenn es noch dunkel ist bis es hell ist .. selbes an Abend. Bis 19:30 Licht und dann schalten die Stripes einzeln ab.

Darum muss der Stall im Auto oder Manuellen Modus arbeiten.

Das force brauch ich für den normalen Schalter .. wenn man in den Stall geht, schaltet man alle Lichter ein. Es soll aber weder manuell per Web noch die Automatik das Licht ändern, solange jemand drinnen ist. Erst wenn man das Licht wieder ausgeschaltet wurde, per Taster an der Wand, arbeitet der Stall wieder wie zuvor.

Auch ein Rauchmelder ist drinnen und soll im Falle das Licht ausschalten und sperren.

Das deutsche Python-Forum

Licht Klasse gestalten

Licht Klasse gestalten

Re: Licht Klasse gestalten

Re: Licht Klasse gestalten

Re: Licht Klasse gestalten

Re: Licht Klasse gestalten

Re: Licht Klasse gestalten

Re: Licht Klasse gestalten

Re: Licht Klasse gestalten

Re: Licht Klasse gestalten

Re: Licht Klasse gestalten

Re: Licht Klasse gestalten