newAM / idasen

Python API and CLI for the ikea IDÅSEN desk.
MIT License
119 stars 20 forks source link

DPG1C Linak controller not supported #369

Closed datbilling closed 8 months ago

datbilling commented 9 months ago

Many people including myself have replaced the original Idasen Desk controller with the upgraded Linak DPG1C controller.

There is an issue when using it with the Home Assistant idasen_desk integration which uses this library though. After a number of hours, moving the desk no longer works, only the current height continues to show. Usually this can be fixed temporarily by disconnecting the controller from HA bluetooth, connecting it to the phone app, moving the desk using the official app to "wake it up again" and then disconnecting and reconnecting to HA.

This has recently been fixed in version 2.1.0 of the project that this one was forked from, rhyst/linak-controller project in https://github.com/rhyst/linak-controller/issues/32

Could the same fix be added here so that it also supports the DPG1C controller please?

newAM commented 9 months ago

I made a PR here with the wakeup command: https://github.com/newAM/idasen/pull/370/files

It looks like the fix is to send a wakeup command before moving: https://github.com/rhyst/linak-controller/issues/32#issuecomment-1782891751

It is easy to reproduce this? Are you able to test that inserting a wakeup before moving solves the problem?

datbilling commented 9 months ago

Brilliant, thank you very much!

I've only used the HA idasen_desk integration which uses this, so can't test it that way yet.

I will install this directly and see if I can reproduce the issue, it should just require leaving it connected for a number of hours and then attempting to move the desk, but only seeing the current height is still showing but it doesn't move.

I could then try the same after installing it from the add-wakeup branch to see if it no longer happens. Please let me know if you see anything wrong with this method.

datbilling commented 9 months ago

I tested this out and unfortunately it doesn't seem to work.

To install the new version, I ran these commands:

git clone https://github.com/newAM/idasen.git
cd idasen
git fetch origin pull/370/head:pr-370
git checkout pr-370
python3 -m pip install -e ./
idasen init
idasen pair
idasen height
idasen stand

I believe that is the correct way to run the new code.

While the controller was already "sleeping", I could get the current height but when attempting to move it to the "stand" position, the command just hangs with no output.

Moving it with the linak-controller tool with the version that has the fix did work however. The idasen stand command then did work because the linak-controller had already awoken it.

Is there anything else I can test?

newAM commented 9 months ago

Sorry for the slow reply, work got crazy for a bit and I forgot about this. The next few months are going to keep being crazy for me, I won't have as much time as I would like for this. I'll be happy to review PRs if someone fixes this before then!

datbilling commented 9 months ago

No problem, thanks for following up on this. I'm not sure what else I can do at this point, I believe I tested the new code correctly but it still doesn't wake the controller.

I hope you don't mind me asking you @kaml123 and @rhyst as I know this is a fork of the linak-controller project, but would you be willing to offer any help on applying the fix for the DPG1C controller here please?

kaml123 commented 9 months ago

Hi @datbilling and @newAM In pull request I found 2 issues. I put some comments there.

datbilling commented 9 months ago

Hi @kaml123, thank you very much for taking a look at this! I created PR #381 with the UUID_DPG & _COMMAND_WAKEUP corrections you commented on. Unfortunately it still doesn't wake the controller though.

I wasn't sure how exactly to implement the 3rd comment though?

I also propose to add self.wakeup() inside async def connect(self):

I tried simply adding self.wakeup() on line 132 under line async def connect(self): but it resulted in an error: idasen\__init__.py:132: RuntimeWarning: coroutine 'IdasenDesk.wakeup' was never awaited self.wakeup() RuntimeWarning: Enable tracemalloc to get the object allocation traceback

kaml123 commented 9 months ago

Hi @datbilling,

I tried simply adding self.wakeup() on line 132 under line async def connect(self): but it resulted in an error: idasen__init__.py:132: RuntimeWarning: coroutine 'IdasenDesk.wakeup' was never awaited self.wakeup() RuntimeWarning: Enable tracemalloc to get the object allocation traceback

Please try changing to:

        while True:
            try:
                await self._client.connect()
                await self.wakeup()
                return
datbilling commented 9 months ago

@kaml123 I added that but received SyntaxError: expected 'except' or 'finally' block. So I instead added await self.wakeup() to the already existing while True: on line 146. I hope there's no issue with that?

I tested it and it works! The desk moved to the stand position when the controller was sleeping. Thank you so much for your help with this.

@newAM could you please approve PR #381 if everything is fine with it and then #370 ?

newAM commented 8 months ago

Added in v0.11.0 see this comment for more context on propagating to home-assistant.