grzegorz914 / homebridge-melcloud-control

Homebridge plugin for Mitsubishi Air Conditioner, Heat Pump and Energy Recovery Ventilation, publish as dynamic external platform accessory.
MIT License
49 stars 2 forks source link

checkDeviceInfo scheduled each time a change is made #71

Closed richardalow closed 4 months ago

richardalow commented 4 months ago

In light of the rate limiting issue, I'm seeing the plugin schedule extra calls to melcloud after each state change. I believe it's due to send() calling checkDeviceInfo(), which sets up another loop of checking. So I believe this line is erroneous and eating into our budget, adding up over time:

https://github.com/grzegorz914/homebridge-melcloud-control/blob/cf609c014fc3f10eed5c9a85d7fe7c0cfeb1e477/src/melcloudata.js#L481

I may be wrong though as I don't know NodeJS.

grzegorz914 commented 4 months ago

You are wrong, after call send() the timeout in checkDeviceInfo() is reset and the timer start from 0 again. In other case the checkDeviceInfo() working in loop.

richardalow commented 4 months ago

I should have included the debug logs that made me suspect a problem (I'll copy from #68). I started the plugin with refresh interval of 300s and it was refreshing every 5 minutes as expected (selecting a single unit and just the first debug log line for clarity):

[06/02/2024, 08:28:30] [melcloudcontrol] Air Conditioner, Master bedroom , debug: Info: {
[06/02/2024, 08:28:31] [melcloudcontrol] Air Conditioner, Master bedroom , debug: State: {
[06/02/2024, 08:33:31] [melcloudcontrol] Air Conditioner, Master bedroom , debug: Info: {
[06/02/2024, 08:33:33] [melcloudcontrol] Air Conditioner, Master bedroom , debug: State: {

Then at 8:30 I switched the unit off:

[06/02/2024, 08:30:02] [melcloudcontrol] Air Conditioner Master bedroom  Set power: OFF

You can now see Info and State logs for this unit on 5 minute intervals after turning it off, in addition to the original 5 minute refreshes:

[06/02/2024, 08:35:02] [melcloudcontrol] Air Conditioner, Master bedroom , debug: Info: {
[06/02/2024, 08:35:03] [melcloudcontrol] Air Conditioner, Master bedroom , debug: State: {
[06/02/2024, 08:38:34] [melcloudcontrol] Air Conditioner, Master bedroom , debug: Info: {
[06/02/2024, 08:38:35] [melcloudcontrol] Air Conditioner, Master bedroom , debug: State: {
[06/02/2024, 08:40:03] [melcloudcontrol] Air Conditioner, Master bedroom , debug: Info: {
[06/02/2024, 08:40:04] [melcloudcontrol] Air Conditioner, Master bedroom , debug: State: {
[06/02/2024, 08:43:36] [melcloudcontrol] Air Conditioner, Master bedroom , debug: Info: {
[06/02/2024, 08:43:37] [melcloudcontrol] Air Conditioner, Master bedroom , debug: State: {
[06/02/2024, 08:45:04] [melcloudcontrol] Air Conditioner, Master bedroom , debug: Info: {
[06/02/2024, 08:45:05] [melcloudcontrol] Air Conditioner, Master bedroom , debug: State: {
[06/02/2024, 08:48:37] [melcloudcontrol] Air Conditioner, Master bedroom , debug: Info: {
[06/02/2024, 08:48:38] [melcloudcontrol] Air Conditioner, Master bedroom , debug: State: {

From my reading of the code, the only place that State is logged is (I only have ATA units) https://github.com/grzegorz914/homebridge-melcloud-control/blob/cf609c014fc3f10eed5c9a85d7fe7c0cfeb1e477/src/melcloudata.js#L356 which is immediately after a call to melcloud. So it seems to me that this is adding additional calls.

After commenting out https://github.com/grzegorz914/homebridge-melcloud-control/blob/cf609c014fc3f10eed5c9a85d7fe7c0cfeb1e477/src/melcloudata.js#L481 I don't see these extra logs and can't tell any difference in functionality.

grzegorz914 commented 4 months ago

If you call checkDeviceInfo() after some other function already called it then is no any additional call for this, just reset this timer: https://github.com/grzegorz914/homebridge-melcloud-control/blob/cf609c014fc3f10eed5c9a85d7fe7c0cfeb1e477/src/melcloudata.js#L452 and start it again from 0.

richardalow commented 4 months ago

Maybe this is the wrong place to look, but those debug logs look very much to me like extra calls are being made. Anyway, maybe this is moot, I see in the beta code that you're reworking this so it caches the state anyway.

grzegorz914 commented 4 months ago

I have rebuild the whole code in latest beta, this discussion is irrelevant now.