Add proper responses to all functions #15

Manually merged
sqozz merged 1 commits from feature/return_codes into master 3 years ago
sqozz commented 3 years ago
Owner

Previously, after a login attempt, you had to send a command which requires authentication before you got any feedback if the login was successful or not.
The reason for this was that send() didn't parse the response of a subsequent notification event.

It could also happen that the plug did not respond to a request at all. Therefore we now propagate the success/error in form of a boolean back to the caller from all functions. In addition, login or password change requests set a newly introduced property called authenticated indicating if the object is already authenticated against the real hardware.

Previously, after a login attempt, you had to send a command which requires authentication before you got any feedback if the login was successful or not. The reason for this was that `send()` didn't parse the response of a subsequent notification event. It could also happen that the plug did not respond to a request at all. Therefore we now propagate the success/error in form of a boolean back to the caller from all functions. In addition, login or password change requests set a newly introduced property called `authenticated` indicating if the object is already authenticated against the real hardware.
sqozz reviewed 3 years ago
Poster
Owner

just a minor cleanup btw

just a minor cleanup btw
sqozz closed this pull request 3 years ago
sqozz closed this pull request 3 years ago
cfr34k requested changes 3 years ago
cfr34k left a comment

Nur eine kleine Frage

Nur eine kleine Frage
msg = self.BTLEMessage(self, cmd, payload)
msg.send()
success = msg.send()
self.authenticated = self.authenticated and success

Meinten Sie: or statt and?

So kann man authenticated nur auf true setzen, wenn es schon true ist :D

Gibt es weiter unten nochmal.

Meinten Sie: `or` statt `and`? So kann man `authenticated` nur auf `true` setzen, wenn es schon `true` ist :D Gibt es weiter unten nochmal.
Poster
Owner

Tatsächlich ist das so beabsichtigt weil der response handler weiter unten in 7a9a2949ff/sem6000.py (L235) authenticated auf true setzt. Das passiert nach dem msg.send(). Beides muss erfolgreich sein damit man auch wirklich eingeloggt ist. Denn failed msg.send() kam die Nachricht nie an, setzt der response handler self.authenticated auf False war der Pin falsch.

Tatsächlich ist das so beabsichtigt weil der response handler weiter unten in https://git.geekify.de/sqozz/sem6000/src/commit/7a9a2949ffafa3a671d598cc0b60544c0f518988/sem6000.py#L235 `authenticated` auf `true` setzt. Das passiert nach dem `msg.send()`. Beides muss erfolgreich sein damit man auch wirklich eingeloggt ist. Denn failed `msg.send()` kam die Nachricht nie an, setzt der response handler `self.authenticated` auf `False` war der Pin falsch.
Poster
Owner

und ich hab mich dazu entschlossen hier den Status von msg.send() zurück zu geben damit das alles konsistent ist. Der caller von login() kann dann nochmal die Funktion callen falls False zurück kam bzw. den User informieren der Pin sei falsch wenn zwar login() True zurück gibt danach aber immer noch socket.authenticated == False ist

und ich hab mich dazu entschlossen hier den Status von `msg.send()` zurück zu geben damit das alles konsistent ist. Der caller von `login()` kann dann nochmal die Funktion callen falls `False` zurück kam bzw. den User informieren der Pin sei falsch wenn zwar `login()` `True` zurück gibt danach aber immer noch `socket.authenticated == False` ist

Ah, alles klar. Danke für die Erklärung. Trotzdem wäre ein kurzer Kommentar wohl hilfreich für zukünftige Leser des Codes :)

Ah, alles klar. Danke für die Erklärung. Trotzdem wäre ein kurzer Kommentar wohl hilfreich für zukünftige Leser des Codes :)
The pull request has been manually merged as 7d16b7b33b.
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sqozz/sem6000#15
Loading…
There is no content yet.