Add proper responses to all functions #15

Manually merged
sqozz merged 1 commits from feature/return_codes into master 2020-05-06 11:05:01 +02:00
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 2020-05-05 23:11:53 +02:00
Author
Owner

just a minor cleanup btw

just a minor cleanup btw
sqozz closed this pull request 2020-05-06 11:05:00 +02:00
sqozz closed this pull request 2020-05-06 11:05:01 +02:00
cfr34k requested changes 2020-05-06 19:27:31 +02:00
cfr34k left a comment
Contributor

Nur eine kleine Frage

Nur eine kleine Frage
@ -69,2 +70,3 @@
msg = self.BTLEMessage(self, cmd, payload)
msg.send()
success = msg.send()
self.authenticated = self.authenticated and success
Contributor

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.
Author
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.
Author
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
Contributor

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 :)
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: sqozz/sem6000#15
No description provided.