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

Added device types. #158

Closed
wants to merge 1 commit into from
Closed

Added device types. #158

wants to merge 1 commit into from

Conversation

nklerk
Copy link
Contributor

@nklerk nklerk commented Dec 2, 2018

I need HDMISWITCH and AUDIO for a project.

These are missing:
'AUDIO',
'HDMISWITCH',
'SOUNDBAR',
'TUNER'

The folowing can give issues:
THERMOSTAT, CLIMA

I need HDMISWITCH and AUDIO for a project.

These are missing:
  'AUDIO',
  'HDMISWITCH',
  'SOUNDBAR',
  'TUNER'

The folowing can give issues:
THERMOSTAT, CLIMA
@pfiaux
Copy link
Contributor

pfiaux commented Dec 3, 2018

These types weren't initially added because they have additional requirements. I think we can include them as long as make sure we document the requirements and add validation.

At least that's my assumption, we'll have to check whether that's still true or not. The missing validation should explain issues if you change the SDK to allow some of these type but without adding the required macros to the devices.

@nklerk
Copy link
Contributor Author

nklerk commented Dec 3, 2018

I’m exposing the HDMISWITCH and AUDIO in my implementation without any noticeable issues. Hope that helps. :-)

@pfiaux
Copy link
Contributor

pfiaux commented Dec 5, 2018

HDMISWITCH should already be in the nextbranch, as well as TUNER, although I feel like tuner might rely on an INPUT TUNER 1 as a fallback. I'll double check AUDIO.

@pfiaux
Copy link
Contributor

pfiaux commented Dec 7, 2018

Alright I went through the types and checked for validation and other issues:

  • AUDIO – no issues
  • HDMISWITCH – needs input validation, also already in next version of SDK
  • SOUNDBAR – needs input validation
  • TUNER – already in next version of SDK
  • THERMOSTAT, CLIMA – known issues (Add support for CLIMA & THERMOSTAT devices #114)

I've discussed this with @neophob, the main issues is that this targets the master branch and we're currently focused on the next branch and we want to keep the SDK releases synchronized with the Firmware versions. Other minor points:

  • Input validation is missing (to generate error if there's no INPUT buttons, see validation/deviceType) for HDMISWITCH
  • JSDOC forsetType() should include the added types (devicebuilder.js)

Considering this we decided to close this (you should be able to use your fork of the SDK if you need them with the current firmware/sdk version). That said I will add AUDIO and SOUNDBAR into the next release and credit you for the changes ;) to make sure we're caught up on the SDK compatible types.

@pfiaux pfiaux closed this Dec 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants