Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race condition - Queue behaviour does not guarantee an item will skip the queue and execute immediately #327

Open
5 tasks done
teh-hippo opened this issue Jul 24, 2023 · 1 comment
Labels
bug Something isn't working due to a problem in the CODE

Comments

@teh-hippo
Copy link

Checklist

  • I checked other issues already, but found no answer/solution
  • I checked the documentation and wiki, but found no answer/solution
  • I am running the latest version and the issue still occurs
  • I have enabled DEBUG and collected DEBUG log (be prepared that incomplete bug reports, especially those missing a full LOG, might be ignored)
  • I am sure that this issue is about homebridge-daikin-local (and not about Apple HomeKit, Homebridge, my local network or something unrelated to this plugin)

Describe the bug

I'm very new to NodeJS though, so I could be wrong here.

I've been trying to add functionality to this plugin for zones in #205.
To do this, when getModelInfo() is called, I need to additional request from the unit which zones have been created. Zones are a Daikin term that, in my case, refers to a group of rooms the central heating is enabled for. I have a zone for the bedrooms. When enabled, fans in the central unit will activate and intentionally push air into those zones. I've been trying to copy the way homebridge-daikin-airbase handles it (full credit to them).

So the process would be:

  • getServices() -> getModelInfo()
    ** getModelInfo() -> sendGetRequest(get_model_info), which queues retrieves the model and other aspects.
    ** getModelInfo() -> sendGetRequest(get_basic_info), which queues retrieves firmware.
    ** getModelInfo() -> sendGetRequest(get_zone_settings), my zone retrieval part.
  • getService() then expects to use results from the first call: get_model_info.

Essentially, those options are not guaranteed to run before continuing with the rest of the getServices() function.

There is an additional issue with the way the queue works with the way skipQueue has been implemented. I tried to use this instead, thinking it'd provide back a result.

skipQueue: true will result in the action being prepended. The issue though, is that if the queue is already running, the operation doesn't begin until the current one completes. In a way this is skipping other operations waiting, but it doesn't mean it will run any faster. It doesn't offer a way to prevent a call from going into the queue to begin with, and run anyway.

I'm not sure how you want to fix it, but I think it's highly likely that the getModelInfo(), on a slow network or with a poorly behaving Daikin device, would not provide its info back before the model info is used.

Screenshots

If applicable, add screenshots to help explain your problem.
To add a screenshot, just drag 'n drop a screenshot file into this editor window.

Additional context

@teh-hippo teh-hippo added the bug Something isn't working due to a problem in the CODE label Jul 24, 2023
@cbrandlehner
Copy link
Owner

@teh-hippo thanks for your feedback. I created this plugin project and I am trying to maintain it.
The concept of queues was added to the project by @fabiandev in an attempt to fix a problem with Daikin devices not responding in time when making too many requests in a short period of time.

I am not experienced with NodeJS either but I can try to merge a pull request if the new code fixes a problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working due to a problem in the CODE
Projects
None yet
Development

No branches or pull requests

2 participants