kovapatrik / homebridge-midea-platform

Homebridge plugin for Midea devices
https://www.npmjs.com/package/homebridge-midea-platform
Apache License 2.0
26 stars 2 forks source link

Add dehumidifier support #14

Closed dkerr64 closed 11 months ago

dkerr64 commented 1 year ago

Would you like me to work on adding support for dehumidifier? I am looking at your AC code and would model dehumidifier after that. I have one of the Midea "cube" dehumidifiers to test with. It is a fair amount of work and I don't want to start if someone else is already doing it.

Let me know and I'll get started.

Thanks.

kovapatrik commented 1 year ago

Of course you can implement it, you are most welcome to do so. But don't use my AC code as a sample (for the accessory part you can use it), instead lookup the repository I've linked in the README. There you can find the Python implementation of the dehumidifer, so you need to write that code in TypeScript (I did the same with the air conditioner, you can see this if you compare the two.)

dkerr64 commented 1 year ago

Thanks for the pointer, I'll look at the python implementation.

dkerr64 commented 1 year ago

Working on this here so far I have the device support implemented, debug log shows that it gets status from the device. Next step is to add homebridge accessory.

kovapatrik commented 1 year ago

Looking good so far!

Btw, if you have a better idea how to do the AccessoryFactory, just add it to your code. I was only able to got through the compiler by explicitly casting the device.

dkerr64 commented 1 year ago

Can you explain what how the messaging to/from devices is supposed to work? In BaseAccessory I see two timers, one for 10 seconds another at 30 seconds. But I don't understand why both are needed. Also, I am not consistently getting data back from the device on refreshing status. If you look at the log below, most times I refresh status I get a single "type 3" message back which includes no data. Occasionally I get a "type 5" message which does contain data.

I've pushed accessory code into the branch, but it is not working yet... suspecting sending/receiving data from the device not working as it should.

[9/26/2023, 8:37:32 PM] [homebridge-midea-platform] [net_a1_363A] Refreshing status...
[9/26/2023, 8:37:35 PM] [homebridge-midea-platform] Got message:
{"HEADER_LENGTH":10,"header":{"type":"Buffer","data":[170,35,161,0,0,0,0,0,3,3]},"device_protocol_version":3,"message_type":3,"device_type":161,"_body":{"data":{"type":"Buffer","data":[200,0,1,80,127,127,0,40,0,0,0,0,0,0,0,50,59,92,0,0,0,0,0,7,55]}},"body_type":200,"message":{"type":"Buffer","data":[170,35,161,0,0,0,0,0,3,3,200,0,1,80,127,127,0,40,0,0,0,0,0,0,0,50,59,92,0,0,0,0,0,7,55,240]}}
[9/26/2023, 8:38:02 PM] [homebridge-midea-platform] [net_a1_363A] Refreshing status...
[9/26/2023, 8:38:05 PM] [homebridge-midea-platform] Got message:
{"HEADER_LENGTH":10,"header":{"type":"Buffer","data":[170,35,161,0,0,0,0,0,3,3]},"device_protocol_version":3,"message_type":3,"device_type":161,"_body":{"data":{"type":"Buffer","data":[200,0,1,80,127,127,0,40,0,0,0,0,0,0,0,50,59,92,0,0,0,0,0,8,118]}},"body_type":200,"message":{"type":"Buffer","data":[170,35,161,0,0,0,0,0,3,3,200,0,1,80,127,127,0,40,0,0,0,0,0,0,0,50,59,92,0,0,0,0,0,8,118,176]}}
[9/26/2023, 8:38:32 PM] [homebridge-midea-platform] [net_a1_363A] Refreshing status...
[9/26/2023, 8:38:35 PM] [homebridge-midea-platform] Got message:
{"HEADER_LENGTH":10,"header":{"type":"Buffer","data":[170,35,161,0,0,0,0,0,3,3]},"device_protocol_version":3,"message_type":3,"device_type":161,"_body":{"data":{"type":"Buffer","data":[200,0,1,80,127,127,0,40,0,0,0,0,0,0,0,50,59,92,0,0,0,0,0,9,40]}},"body_type":200,"message":{"type":"Buffer","data":[170,35,161,0,0,0,0,0,3,3,200,0,1,80,127,127,0,40,0,0,0,0,0,0,0,50,59,92,0,0,0,0,0,9,40,253]}}
[9/26/2023, 8:39:02 PM] [homebridge-midea-platform] [net_a1_363A] Refreshing status...
[9/26/2023, 8:39:05 PM] [homebridge-midea-platform] Got message:
{"HEADER_LENGTH":10,"header":{"type":"Buffer","data":[170,35,161,0,0,0,0,0,3,3]},"device_protocol_version":3,"message_type":3,"device_type":161,"_body":{"data":{"type":"Buffer","data":[200,0,1,80,127,127,0,40,0,0,0,0,0,0,0,50,59,92,0,0,0,0,0,10,202]}},"body_type":200,"message":{"type":"Buffer","data":[170,35,161,0,0,0,0,0,3,3,200,0,1,80,127,127,0,40,0,0,0,0,0,0,0,50,59,92,0,0,0,0,0,10,202,90]}}
[9/26/2023, 8:39:32 PM] [homebridge-midea-platform] [net_a1_363A] Refreshing status...
[9/26/2023, 8:39:36 PM] [homebridge-midea-platform] Got message:
{"HEADER_LENGTH":10,"header":{"type":"Buffer","data":[170,35,161,0,0,0,0,0,3,3]},"device_protocol_version":3,"message_type":3,"device_type":161,"_body":{"data":{"type":"Buffer","data":[200,0,1,80,127,127,0,40,0,0,0,0,0,0,0,50,59,92,0,0,0,0,0,11,148]}},"body_type":200,"message":{"type":"Buffer","data":[170,35,161,0,0,0,0,0,3,3,200,0,1,80,127,127,0,40,0,0,0,0,0,0,0,50,59,92,0,0,0,0,0,11,148,143]}}
[9/26/2023, 8:39:36 PM] [homebridge-midea-platform] Got message:
{"HEADER_LENGTH":10,"header":{"type":"Buffer","data":[170,35,161,0,0,0,0,0,3,5]},"device_protocol_version":3,"message_type":5,"device_type":161,"_body":{"data":{"type":"Buffer","data":[160,0,1,80,127,127,0,40,0,0,0,0,0,0,0,50,59,92,0,0,0,0,0,11,125]},"power":false,"mode":0,"fan_speed":80,"target_humidity":40,"child_lock":false,"anion":false,"tank":0,"water_level_set":50,"current_humidity":59,"current_temperature":21,"swing":false},"body_type":160,"message":{"type":"Buffer","data":[170,35,161,0,0,0,0,0,3,5,160,0,1,80,127,127,0,40,0,0,0,0,0,0,0,50,59,92,0,0,0,0,0,11,125,204]}}
[9/26/2023, 8:39:36 PM] [homebridge-midea-platform] [net_a1_363A] Setting local POWER to false
[9/26/2023, 8:39:36 PM] [homebridge-midea-platform] [net_a1_363A] Setting local CHILD_LOCK to false
[9/26/2023, 8:39:36 PM] [homebridge-midea-platform] [net_a1_363A] Setting local MODE to 0
[9/26/2023, 8:39:36 PM] [homebridge-midea-platform] [net_a1_363A] Setting local FAN_SPEED to 80
[9/26/2023, 8:39:36 PM] [homebridge-midea-platform] [net_a1_363A] Setting local SWING to false
[9/26/2023, 8:39:36 PM] [homebridge-midea-platform] [net_a1_363A] Setting local TARGET_HUMIDITY to 40
[9/26/2023, 8:39:36 PM] [homebridge-midea-platform] [net_a1_363A] Setting local ANION to false
[9/26/2023, 8:39:36 PM] [homebridge-midea-platform] [net_a1_363A] Setting local TANK to 0
[9/26/2023, 8:39:36 PM] [homebridge-midea-platform] [net_a1_363A] Setting local WATER_LEVEL_SET to 50
[9/26/2023, 8:39:36 PM] [homebridge-midea-platform] [net_a1_363A] Setting local CURRENT_HUMIDITY to 59
[9/26/2023, 8:39:36 PM] [homebridge-midea-platform] [net_a1_363A] Setting local CURRENT_TEMPERATURE to 21
[9/26/2023, 8:40:02 PM] [homebridge-midea-platform] [net_a1_363A] Refreshing status...
[9/26/2023, 8:40:05 PM] [homebridge-midea-platform] Got message:
{"HEADER_LENGTH":10,"header":{"type":"Buffer","data":[170,35,161,0,0,0,0,0,3,3]},"device_protocol_version":3,"message_type":3,"device_type":161,"_body":{"data":{"type":"Buffer","data":[200,0,1,80,127,127,0,40,0,0,0,0,0,0,0,50,59,92,0,0,0,0,0,12,23]}},"body_type":200,"message":{"type":"Buffer","data":[170,35,161,0,0,0,0,0,3,3,200,0,1,80,127,127,0,40,0,0,0,0,0,0,0,50,59,92,0,0,0,0,0,12,23,11]}}
[9/26/2023, 8:40:32 PM] [homebridge-midea-platform] [net_a1_363A] Refreshing status...
[9/26/2023, 8:40:35 PM] [homebridge-midea-platform] Got message:
{"HEADER_LENGTH":10,"header":{"type":"Buffer","data":[170,35,161,0,0,0,0,0,3,3]},"device_protocol_version":3,"message_type":3,"device_type":161,"_body":{"data":{"type":"Buffer","data":[200,0,1,80,127,127,0,40,0,0,0,0,0,0,0,50,59,92,0,0,0,0,0,13,73]}},"body_type":200,"message":{"type":"Buffer","data":[170,35,161,0,0,0,0,0,3,3,200,0,1,80,127,127,0,40,0,0,0,0,0,0,0,50,59,92,0,0,0,0,0,13,73,216]}}
[9/26/2023, 8:41:02 PM] [homebridge-midea-platform] [net_a1_363A] Refreshing status...
[9/26/2023, 8:41:05 PM] [homebridge-midea-platform] Got message:
{"HEADER_LENGTH":10,"header":{"type":"Buffer","data":[170,35,161,0,0,0,0,0,3,3]},"device_protocol_version":3,"message_type":3,"device_type":161,"_body":{"data":{"type":"Buffer","data":[200,0,1,80,127,127,0,40,0,0,0,0,0,0,0,50,59,92,0,0,0,0,0,14,171]}},"body_type":200,"message":{"type":"Buffer","data":[170,35,161,0,0,0,0,0,3,3,200,0,1,80,127,127,0,40,0,0,0,0,0,0,0,50,59,92,0,0,0,0,0,14,171,117]}}
[9/26/2023, 8:41:32 PM] [homebridge-midea-platform] [net_a1_363A] Refreshing status...
[9/26/2023, 8:41:35 PM] [homebridge-midea-platform] Got message:
{"HEADER_LENGTH":10,"header":{"type":"Buffer","data":[170,35,161,0,0,0,0,0,3,3]},"device_protocol_version":3,"message_type":3,"device_type":161,"_body":{"data":{"type":"Buffer","data":[200,0,1,80,127,127,0,40,0,0,0,0,0,0,0,50,59,92,0,0,0,0,0,15,245]}},"body_type":200,"message":{"type":"Buffer","data":[170,35,161,0,0,0,0,0,3,3,200,0,1,80,127,127,0,40,0,0,0,0,0,0,0,50,59,92,0,0,0,0,0,15,245,42]}}
kovapatrik commented 1 year ago

The two timers were copied from the mentioned Python repository. There they used 1 timer for sending a heartbeat (to not lose the connection), and an other one for refreshing status. You are right, one timer could be enough (like refreshing status every 10secs).

Your two types of messages are there the same reason: type 3 is a heartbeat response, type 5 is a refresh status response.

dkerr64 commented 1 year ago

I'll look into whether both timers are needed once I have things working.

In the meantime I need to solve why I'm not getting status data every time I request it. We're requesting it every 30 seconds. I'm getting type 5 messages much less frequently, in fact looking at logs from last 12 hours it appears it is only sent if there is a change in value, e.g. current humidity is different. That might sound okay, but when we do first initialization we need to get current status and that is not coming back.

I will look at how the messages differ from the other implementation that is working and see if I can figure it out.

kovapatrik commented 1 year ago

You could try handling all the responses as GeneralMessageBody, so just comment out this if case. I've also got problems with these NewProtocolXY things...

dkerr64 commented 1 year ago

The problem was a coding error in my conversion from python to javascript. With that fixed, it is correctly receiving device status from my dehumidifier. I do not have changing settings working yet (e.g. simply powering on the device).

As for the two timers. I decided to comment out the heartbeat timer and set the refresh status timer to 10 seconds. When I did that I found that I needed to tell refresh_status to wait for a reply.

It's possible that the way it was originally designed to work is that every 30 seconds a request to refresh status was sent, but that timer did not wait for a reply. The other timer, running every 10 seconds does read data from the network socket and so it would retrieve the status reply. Why design it this way? Well if the device pushes notifications on status changes without first receiving a query then updates would be received immediately (well, every 10 seconds) rather than waiting for another query (which would be every 30 seconds). I have not tested this yet, but it does feel fairly elegant though perhaps over-engineered (for this device type).

Next step is to get sending commands to the device working.

dkerr64 commented 1 year ago

Setting power on/off now working. I suspect setting other value also works. Problem was, again, error when I copied from the original python... I set the message body type to 0x40 when it should have been 0x48.

While debugging also noticed that coding value as boolean is not valid way to convert integer to boolean in javascript. It removes the typescript error but does not change a 1 to true. Correct way is !!value. You should update your AC code.

dkerr64 commented 1 year ago

Now, we do still have the problem of how to handle networking. The two timers doesn't feel right. But we do need to have a "thread" somewhere that is constantly listening on the socket. Currently the code to send a message to the device does not listen for a reply. That acknowledgement packet gets received next time the code does listen (in my current example, when next refresh_status is done.

I am not sure that BaseAccessory is the right place to do this, it should be attached to the MideaDevice code and independent of the homebridge accessory code. I think on successful discovery/authentication a listener should be setup to receive messages from the device. I'll give it some thought.

dkerr64 commented 1 year ago

Yup. We need to implement a loop like in midea_ac_lan. They also include the refresh in that loop, which would be better place for it than BaseAccessory.

I will implement similar to how they did it.

dkerr64 commented 1 year ago

I've implemented the network listener loop in MideaDevice and simplified promiseSocket. Needs a lot more testing for error situations, but I like where things are at. I can see now why there needs to be a heartbeat as without it the socket gets closed (presumably for inactivity). But there are protections in place to open it back up again if it does close.

More to do, but good progress I think.

dkerr64 commented 1 year ago

Working dehumidifier is in this branch

Not ready to merge yet. I need to update air conditioner device/accessory to use the new device refresh mechanism and networking.

kovapatrik commented 1 year ago

Great job! I didn't have the chance to look on, but isn't it possible to get the broadcast IP without using Netmask? If it's not, can you please use ES6 import style instead of require?

dkerr64 commented 1 year ago

Great job! I didn't have the chance to look on, but isn't it possible to get the broadcast IP without using Netmask? If it's not, can you please use ES6 import style instead of require?

Yes, I found a method to generate the broadcast address. I have removed dependency on netmask in this commit.

dkerr64 commented 1 year ago

My develop branch now has air conditioner code updated. I do not have a AC device to test with so would be grateful if you can do that. Place that might need attention is here, in the callback function where changes are updated with Homebridge/Homekit. The switch/case statement may not be catching all possible values. Any that are received and not handed are logged at warning level.

dkerr64 commented 1 year ago

I have further updates I want to make to the platform. Right now every time the plugin restarts it logs into the Midea cloud. This is not necessary (after done once) so I want to change the discovery code to be more like that I had implemented here.

This has many advantages, allowing for completely local LAN-based operation. The only time a login to Midea cloud should be required is if new Midea devices are added to the network (in order to retrieve a token/key pair).

There is one downside... discovery needs to be allowed to complete before initializing each device. Right now they initialize as soon as they are found. With 3 retries at 3 seconds apart that is a 9 second discovery process which needs to complete (unless devices are pre-configured in the config file, in which case discovery can be skipped).

dkerr64 commented 1 year ago

I have further updates I want to make to the platform. Right now every time the plugin restarts it logs into the Midea cloud. This is not necessary (after done once) so I want to change the discovery code to be more like that I had implemented here.

This has many advantages, allowing for completely local LAN-based operation. The only time a login to Midea cloud should be required is if new Midea devices are added to the network (in order to retrieve a token/key pair).

There is one downside... discovery needs to be allowed to complete before initializing each device. Right now they initialize as soon as they are found. With 3 retries at 3 seconds apart that is a 9 second discovery process which needs to complete (unless devices are pre-configured in the config file, in which case discovery can be skipped).

I found a way to optimize login without having to wait until all discovery broadcasts are done. Implemented in my develop branch.

Benigans commented 1 year ago

I have further updates I want to make to the platform. Right now every time the plugin restarts it logs into the Midea cloud. This is not necessary (after done once) so I want to change the discovery code to be more like that I had implemented here. This has many advantages, allowing for completely local LAN-based operation. The only time a login to Midea cloud should be required is if new Midea devices are added to the network (in order to retrieve a token/key pair). There is one downside... discovery needs to be allowed to complete before initializing each device. Right now they initialize as soon as they are found. With 3 retries at 3 seconds apart that is a 9 second discovery process which needs to complete (unless devices are pre-configured in the config file, in which case discovery can be skipped).

I found a way to optimize login without having to wait until all discovery broadcasts are done. Implemented in my develop branch.

Hi ! I have a dehumidifier that I would like to use in homebridge. I can test your version of the plugin if it's possible.

dkerr64 commented 1 year ago

Hi ! I have a dehumidifier that I would like to use in homebridge. I can test your version of the plugin if it's possible.

@Benigans thanks for the offer, I would like that. If @kovapatrik is ready to merge this into his mainline and publish then I will create a pull request for him to do that, that would be easiest way for you to get it.

Alternatively you would have to setup a Homebridge development environment and clone a copy of my code and build it. I'm not sure I would recommend that unless you're already familiar with Github, npm and Homebridge development. It would be better to merge and publish.

kovapatrik commented 1 year ago

Sorry, I didn't have time to test it until today. The air conditioner is almost working, there are a few bugs. Can you please create a pull request so I can fix those bugs?

Also it would be nice to include the fix for #10 in the next release. If you have time, can you please look for bugs?

Edit: I've just checked on the log again, and it seems like the socket object gets an error frequently (like my homebridge service is on a restart loop now):

[10/5/2023, 1:25:43 PM] [homebridge-midea-platform] [Living Room AC] Refreshing status...

/home/pi/homebridge-midea-platform/src/core/MideaUtils.ts:86
      throw Error(`Socket error: ${err}`);
            ^
Error: Socket error: Error: This socket has been ended by the other party
    at Socket.<anonymous> (/home/pi/homebridge-midea-platform/src/core/MideaUtils.ts:86:13)
    at Socket.emit (node:events:526:35)
    at emitErrorNT (node:internal/streams/destroy:151:8)
    at emitErrorCloseNT (node:internal/streams/destroy:116:3)
    at processTicksAndRejections (node:internal/process/task_queues:82:21)
[10/5/2023, 1:25:43 PM] [homebridge-midea-platform] Child bridge process ended

I think when the socket gets an error, it should be reinitialized instead of throwing an error.

dkerr64 commented 1 year ago

We need to trace back to see if the promiseSocket destroy() is called. There is no drug trace in that function, we should add. When testing I saw the socket close, possibly by the end device, if heartbeat was not being sent.

In the meantime I will do a pull request anyway so we can get this in mainline if you want it.

dkerr64 commented 1 year ago

Note... I added ability to add a verbose=true statement to config file (see MideaDevice.ts). While debugging I found it helpful to trace all the network traffic, but once I got it working felt that it filled up the log too much so I put those debug statements behind a test for verbose flag. You might want to set that to true wile working on above error.

kovapatrik commented 1 year ago

I've used the logger.debug function for these outputs which will only show if you run your Homebridge instance with the debug flag. I think it's a better approach than creating a hardcoded flag.

dkerr64 commented 1 year ago

TODO: Some things I'd like to add.

  1. Add a forceLogin boolean to config file. Currently we only login to MideaCloud when adding a new device. But it may be desirable to always (or occasionally) login to retrieve a new Token/Key pair from the device rather than use whatever is cached in Homebridge accessory cache.
  2. Add a refreshInterval=\<seconds> to config file. Right now it is hard coded to 30 seconds. I think it should be configurable. Based on my testing this is unnecessarily frequent as the device will push updates as they occur. I also just have a thing against hardcoding things like this in the code. Heartbeat is similarly hardcoded to 10 seconds, but I am in two minds as to whether this should be configurable.
dkerr64 commented 1 year ago

I've used the logger.debug function for these outputs which will only show if you run your Homebridge instance with the debug flag. I think it's a better approach than creating a hardcoded flag.

I still use logger.debug. But some of these I have placed behind that flag because it was just getting too much.

kovapatrik commented 1 year ago

Ah okay. I think we can spam the debug output though, it will be only seen by developers.

dkerr64 commented 1 year ago

Ah okay. I think we can spam the debug output though, it will be only seen by developers.

Then change the default from false to true.

dkerr64 commented 1 year ago

I can't look at issue #10 for a few days but will try over the weekend

kovapatrik commented 1 year ago

TODO: Some things I'd like to add.

1. Add a **forceLogin** boolean to config file.  Currently we only login to MideaCloud when adding a new device.  But it may be desirable to always (or occasionally) login to retrieve a new Token/Key pair from the device rather than use whatever is cached in Homebridge accessory cache.

2. Add a **refreshInterval=<seconds>** to config file.  Right now it is hard coded to 30 seconds.  I think it should be configurable.  Based on my testing this is unnecessarily frequent as the device will push updates as they occur.  I also just have a thing against hardcoding things like this in the code.  Heartbeat is similarly hardcoded to 10 seconds, but I am in two minds as to whether this should be configurable.

If I undestand correctly, once you have a token/key it should always work, these won't expire.

dkerr64 commented 1 year ago

Ah okay. I think we can spam the debug output though, it will be only seen by developers.

Even for developers, at least this developer (me) there can be too much logging. It is fairly common to have multiple levels of debug logging (think commands that allow -v and -vv and -vvv). Anyway, I have changed the default to output full logging, need to add "verbose": false to config to suppress the network traffic logging.

dkerr64 commented 1 year ago

TODO: Some things I'd like to add.

1. Add a **forceLogin** boolean to config file.  Currently we only login to MideaCloud when adding a new device.  But it may be desirable to always (or occasionally) login to retrieve a new Token/Key pair from the device rather than use whatever is cached in Homebridge accessory cache.

2. Add a **refreshInterval=<seconds>** to config file.  Right now it is hard coded to 30 seconds.  I think it should be configurable.  Based on my testing this is unnecessarily frequent as the device will push updates as they occur.  I also just have a thing against hardcoding things like this in the code.  Heartbeat is similarly hardcoded to 10 seconds, but I am in two minds as to whether this should be configurable.

If I undestand correctly, once you have a token/key it should always work, these won't expire.

Yes, I believe that to be correct too. But you never know, there might be a situation that causes old token/keys to become invalid.

kovapatrik commented 1 year ago

Ah okay. I think we can spam the debug output though, it will be only seen by developers.

Even for developers, at least this developer (me) there can be too much logging. It is fairly common to have multiple levels of debug logging (think commands that allow -v and -vv and -vvv). Anyway, I have changed the default to output full logging, need to add "verbose": false to config to suppress the network traffic logging.

I can see why you want to hide some logs, but I think it's not so evident that you have to recompile the whole project just to see more verbose output messages.

dkerr64 commented 12 months ago

TODO: Some things I'd like to add.

1. Add a **forceLogin** boolean to config file.  Currently we only login to MideaCloud when adding a new device.  But it may be desirable to always (or occasionally) login to retrieve a new Token/Key pair from the device rather than use whatever is cached in Homebridge accessory cache.

2. Add a **refreshInterval=<seconds>** to config file.  Right now it is hard coded to 30 seconds.  I think it should be configurable.  Based on my testing this is unnecessarily frequent as the device will push updates as they occur.  I also just have a thing against hardcoding things like this in the code.  Heartbeat is similarly hardcoded to 10 seconds, but I am in two minds as to whether this should be configurable.

If I undestand correctly, once you have a token/key it should always work, these won't expire.

Yes, I believe that to be correct too. But you never know, there might be a situation that causes old token/keys to become invalid.

While investigating why NetHome Plus is not working, I found it necessary to repeatedly try and login. So I implemented a forceLogin setting.

dkerr64 commented 12 months ago

Ah okay. I think we can spam the debug output though, it will be only seen by developers.

Even for developers, at least this developer (me) there can be too much logging. It is fairly common to have multiple levels of debug logging (think commands that allow -v and -vv and -vvv). Anyway, I have changed the default to output full logging, need to add "verbose": false to config to suppress the network traffic logging.

I can see why you want to hide some logs, but I think it's not so evident that you have to recompile the whole project just to see more verbose output messages.

Yes, as this is developer specific feature, implementing as a simple compile time boolean would be sufficient. Same for forceLogin.

kovapatrik commented 12 months ago

You mean runtime config? Like adding a checkbox into the config scheme? I can accept that, as long as we have a solution which is not require recompiling. Note that if there is a bug a normal user might need to show us a more verbose output.

kovapatrik commented 12 months ago

I've checked the socket problem: it looks like neither close or destroy is called before the error is thrown, so the connection got closed during a read or write command I suppose. What do you suggest we should do? I still think we need to manually reconnect to the device when an error is occured.

dkerr64 commented 12 months ago

You mean runtime config? Like adding a checkbox into the config scheme? I can accept that, as long as we have a solution which is not require recompiling. Note that if there is a bug a normal user might need to show us a more verbose output.

I will add to the config schema. Thank you.

dkerr64 commented 12 months ago

I've checked the socket problem: it looks like neither close or destroy is called before the error is thrown, so the connection got closed during a read or write command I suppose. What do you suggest we should do? I still think we need to manually reconnect to the device when an error is occured.

Yes we need to attempt recovery and reconned. There is some code to do this already in the run() function in MideaDevice but it depends on promiseSocket.destroyed having been set... which is probably not set if the remote closes the socket, or it is closed under an error condition. I will think about how recovery is best handled.

kovapatrik commented 12 months ago

If it depends on the destroyed state the easiest solution might be to just destroy the socket on error and log the error out.

A thought about the registerUpdate solution: I guess you copied this from the midea-ac-lan repository (if I remember correctly it has a similar solution), but wouldn't be it simpler if the MideaDevice will be an EventEmitter and it simply emits the changes attributes?

dkerr64 commented 12 months ago

If it depends on the destroyed state the easiest solution might be to just destroy the socket on error and log the error out.

A thought about the registerUpdate solution: I guess you copied this from the midea-ac-lan repository (if I remember correctly it has a similar solution), but wouldn't be it simpler if the MideaDevice will be an EventEmitter and it simply emits the changes attributes?

Yes, that occurred to me after I had implemented it. An event emit would be more elegant way to handle it. I did indeed model after midea-ac-lan.

dkerr64 commented 12 months ago

You mean runtime config? Like adding a checkbox into the config scheme? I can accept that, as long as we have a solution which is not require recompiling. Note that if there is a bug a normal user might need to show us a more verbose output.

I will add to the config schema. Thank you.

I have pushed update to config schema.

dkerr64 commented 12 months ago

Can you try this change to see if it fixes the 'This socket has been ended by the other party' error.

The theory here is that on remote party closing the connection we can recover by destroying the socket and recreating it. That will take place in MideaDevice. I made no attempt to check the cause of the error, so it will take this action for all error conditions, not sure if that is good or bad.

I commented out the error throw because I don't know where it would be caught... Can you test with it uncommented please. I hope it will get caught in MideaDevice here, in which case we should probably leave it in... and the error handling code will helpfully log an error for us, and continue to recreate the socket just as above... though we might want to abbreviate how much is logged (unless verbose true).

Another thought I had was to increase the timeout from 1 second to something longer, say 5 seconds. If we do that then timeout_counter > 120should probably be changed too.

Thanks

kovapatrik commented 12 months ago

I set the timeout to 1 seconds, because in case of my AC, all message getting timeout, so if I set the timeout to 5 secs, then I have to wait 5 seconds after sending any message before I receive it.

dkerr64 commented 11 months ago

I set the timeout to 1 seconds, because in case of my AC, all message getting timeout, so if I set the timeout to 5 secs, then I have to wait 5 seconds after sending any message before I receive it.

I see the same thing. This suggests that something else is broken in the communication to/from devices. The read()function in MideaUtils resolves the promise on data > size requested, error, end or timeout. It doesn't feel right to rely on timeout but maybe that is how it has to be. I will look at it.

dkerr64 commented 11 months ago

I have updated the PromiseSocket and networking in MideaDevice to make it work better.

Data read from the device is returned immediately without waiting for timeout. I removed the size parameter from read() as it is not used and was previously used incorrectly. There is in fact no read method in the underlying net.Socket, it is a feature of the promise wrap. I removed readAll() as it was never used. I have added console logging to error and close socket events... Homebridge logger is not available in PromiseSocket and rather than add it I decided to just use console logging.

I tested recovery from socket close by setting the heartbeat interval to a longer time (without heartbeat or refresh status the socket closes after ~30 seconds).

I think this should work well. Please test it.

dkerr64 commented 11 months ago

If it depends on the destroyed state the easiest solution might be to just destroy the socket on error and log the error out. A thought about the registerUpdate solution: I guess you copied this from the midea-ac-lan repository (if I remember correctly it has a similar solution), but wouldn't be it simpler if the MideaDevice will be an EventEmitter and it simply emits the changes attributes?

Yes, that occurred to me after I had implemented it. An event emit would be more elegant way to handle it. I did indeed model after midea-ac-lan.

This is now implemented with EventEmitter.

kovapatrik commented 11 months ago

Great work! I've created a PR to your fork in the meantime.

dkerr64 commented 11 months ago

I have merged all your changes into my branch.