Skip to content

Conversation

@rycardo
Copy link

@rycardo rycardo commented Aug 22, 2017

It appears the code to support 'treatAsFanIds' was added to this file,
but the treatAsFanIds array and the lines to read from the array were not
added yet. I added the code to read the treatAsFansId array and the call to create
the fan accessories from it.

I also put the example and definition "treatAs..." lines in
alphabetical order.

I didn't update the config.json file, as I'm not sure if there is a reason you didn't add the missing code. I'm using fans on a handful of Indigo devices, and in 18 hours I haven't had any issues.

My first pull request to this project, I welcome any feedback.

rycardo and others added 4 commits August 22, 2017 12:11
Update Rycardo master branch from webdeck's master branch
It appeared the code to support 'treatAsFanIds' was added to the file,
but the treatAsFanIds array and the lines to read the array were not
added yet. I added the lines of code to read the array and to create
the fan accessories from it.

I also put the example and definition "treatAs..." lines in
alphabetical order.

My first pull request to this project, I welcome any feedback.
Copy link
Owner

@webdeck webdeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't going to work as written because FanAccessory assumes it is communicating with an Indigo device that supports speed control, not just on/off.

@rycardo
Copy link
Author

rycardo commented Aug 22, 2017

Hey - I wondered about that, but I also didn't want to modify your code if it wasn't necessary, and I didn't want to cause actual fan devices to not work correctly. I thought about returning the speed as maximum speed when on (for these treat as devices).

I did add this else if block, which I copied from existing code:
} else if (json.typeSupportsOnOff && this.treatAsFanIds && (this.treatAsFanIds.indexOf(String(json.id)) >= 0)) {
Does that get around your concern?

@webdeck
Copy link
Owner

webdeck commented Aug 22, 2017

The issue is that Homekit will show a speed control and that won't work - any attempt to get/set the speed will result in the error "Accessory does not support rotation speed".

@rycardo
Copy link
Author

rycardo commented Aug 22, 2017

I understand what you are saying now. I had only viewed these devices in Home, and they show as a spinning fan on or off, also previous to changing the code, I edited their details in Home, changing them from Switch to Fan. I could turn them on/off in Home, with no speed control, although I very rarely did as they are on schedules.

Now, when I attempt to turn them on/off, I see what you are saying. Let me give it some more thought.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants