oyooyo / keyble

Command line tools and library for controlling eqiva eQ-3 Bluetooth smart locks
90 stars 28 forks source link

Issues in the new code which removes the hardcoded delays #4

Open Ircama opened 5 years ago

Ircama commented 5 years ago

Thanks for the new version. Unfortunately, I had limited time to test it. It sounds good and really speeds up timing. Here are some glitches:

oyooyo commented 5 years ago

Thank you very much for your very valuable feedback. I really appreciate this.

when Eqiva eQ-3 device is not in range, keyble-sendcommand waits indefinitely for the "active" step: a timeout would be useful.

You're right - ideally, there should be a way to set timeouts for all actions. As a kind of compromise until I find a better solution, there is a small helper function keyble.utils.time_limit now: https://github.com/oyooyo/keyble#timeouts-for-actions

And the keyble-sendcommand tool now got a new --timeout argument. I didn't actually test it yet though.

The meaning of the STATUS_UPDATE_TIME timer is not clear to me; it does not look to control this timeout anyway.

Yes, I didn't initially give a good description. I just changed that:

      --auto_disconnect_time AUTO_DISCONNECT_TIME, -adt AUTO_DISCONNECT_TIME
                            The auto-disconnect time. If connected to the lock, 
                            the connection will be automatically disconnected 
                            after this many seconds of inactivity, in order to 
                            save battery. A value of 0 will deactivate 
                            auto-disconnect (default: 30)
      --status_update_time STATUS_UPDATE_TIME, -sut STATUS_UPDATE_TIME
                            The status update time. If no status information has 
                            been received for this many seconds, automatically 
                            connect to the lock and query the status. A value of 
                            0 will deactivate status updates (default: 600)

lock command works, then the step switches from "active" to "undefined" and subsequently it freezes (control C to break); after interrupting, the status reports "undefined"; this is because my device uses status 3 for signaling the lock position (to test it, I labeled it closed in lock_status_string within keyble.js. After defining this additional status (3: 'closed') and changing the event to wait it in the lock() function (return this.await_event('status:closed')), the lock command completes correctly. Perhaps both events 0 and 3 (locked and closed) should be waited.

Now this is quite interesting/strange. I'm starting to believe that my interpretation of the various status codes is severely wrong. Because personally, I've never witnessed status code 3 so far. And on some web forum, someone else told me that his device never actually reports status code 4 (which I interpreted as "open" so far) when he opens the door; unlike my device, it seems to simply skip this state.

I could really need your help here. Could you maybe play around with the lock a bit, sending a number of "open", "lock", "unlock", "status" commands, write down all the various status code that you witness on your device, and provide your very own interpretation of what you assume these status codes stand for?

open command works, then freezes (control C to break); subsequently, a status query reports "unlocked"; I noticed that the issue can be fixed by setting the default_auto_disconnect_time to 30 seconds instead of 15. The many rotations of my Eqiva eQ-3 device in both directions issued by the open command take more than 15 seconds to complete.

Good point, I just increased the default_auto_disconnect_time to 30 seconds as you suggested and published version 0.1.10.

Ircama commented 5 years ago

new --timeout argument.

The timeout seems to wait too long if I do not specify the --timeout option

I think that

defaultValue: default_status_update_time,

should be:

defaultValue: default_timeout_time,

interpretation of the various status codes

Status 0 and status 4 never occurred in my tests.

This works (notice 3: 'locked',):

lock_status_string = {
          0: 'locked',
          1: 'active',
          2: 'unlocked',
          3: 'locked',
          4: 'open'
        }[lock_status_id];

Anyway, consider that sometimes the device is not able to unlock the door (e.g., it might rarely happen when manually turning many times the handwheel); in such cases, the returning status of an unlock command is lock. (possibly the open operation [or removing and inserting the batteries] resets the correct behaviour.) So, it is better to wait for both locked/unlocked codes to avoid exiting by timeout sometimes.

There is also another particular case: if the unlock position has not yet been registered (this happens just after replacing the battery), the lock operation works (ref. settings of the number of turns in the App configuration), but not the unlock, which does not return the initial active status (3 is returned instead and this also produces the expiration of the timeout). The unlock position is set by the device after pressing the unlock key, when reaching the end of rotation of the cylinder lock (this forces the motor to stop); it can be also obtained manually, by blocking the rotation of the handwheel. After the accomplishment of the registration of the unlock position, the unlock operation works.

increased the default_auto_disconnect_time to 30 seconds This works now.

oyooyo commented 5 years ago

defaultValue: default_status_update_time, should be: defaultValue: default_timeout_time,

Argh, of course. Thanks for pointing out.

Status 0 and status 4 never occurred in my tests.

That's strange. As I told before, some other guy also reported to me that status code 4 did not occur on his lock. But meanwhile it turned out that this was because his lock was sitting on his desk next to him, and when he sent the commands and the lock started rotating, he stopped the movement by pressing one of the buttons, which seems to have cancelled the action or something, so his lock did probably not behave exactly is it would under real conditions. When he stopped the open() movement by blocking the handwheel, simulating how an actual cylinder lock would behave, status 4 did occur on his lock as well.

Anyways, I've meanwhile had another look at the original app, and luckily managed to find an enumeration of the various status codes. I've now simply adopted the names they used: lock_status_string = { 0:'UNKNOWN', 1:'MOVING', 2:'UNLOCKED', 3:'LOCKED', 4:'OPENED', }[lock_status_id]

But you've made me unsure which status codes to accept to consider that an action has succeeded. Intuitevely, I'd consider the open() action to be successful if status "4" was reached, lock() is status code "3" was reached, and unlock if status code "2" was reached.

What do you suggest?

Ircama commented 5 years ago

My homemade setup is specific and the device is free, not driving a cylinder lock, so not stopped by a rotation limit and cannot measure the torque to define the end of the rotation: the handwheel is connected to a little chain that drags the linear travel of a sliding bolt which includes a release spring returning the bolt back to its default position after the dragging. I do manually the initial configuration to define the open and close positions. The effect of this set-up is that the "open" command (not useful for my use case) is not limited and maybe this is the reason why I never get status 4 for it and always get status 2.

As per lock and unlock operations, it is correct for the default behavior to get respectively status 3 and 4 as you write. Anyway I mentioned of particular cases where unlock is not able to go to 2 and remains to 3. Also, there is a case where the unlock will never catch the initial status 1. Ideally these cases could be managed to avoid to indefinitely freeze the call until the timeout; but at the end the already implemented timeout is a decent exit method to handle such specific conditions.