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

Extended Air Conditioning Companion support #169

Merged
merged 8 commits into from
Jan 26, 2018

Conversation

syssi
Copy link
Collaborator

@syssi syssi commented Jan 21, 2018

No description provided.

@syssi syssi changed the title Extended Air Conditioning Companion support WIP: Extended Air Conditioning Companion support Jan 21, 2018
@coveralls
Copy link

Coverage Status

Coverage remained the same at 62.335% when pulling 118acaf on syssi:feature/acpartner-updates into ea756d0 on rytilahti:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 62.335% when pulling 118acaf on syssi:feature/acpartner-updates into ea756d0 on rytilahti:master.

@coveralls
Copy link

coveralls commented Jan 21, 2018

Coverage Status

Coverage increased (+0.4%) to 62.781% when pulling f7c0edb on syssi:feature/acpartner-updates into ea756d0 on rytilahti:master.

@syssi syssi closed this Jan 21, 2018
@syssi syssi reopened this Jan 21, 2018
@syssi syssi requested a review from rytilahti January 21, 2018 14:55
configuration = configuration.replace('mo', operation_mode.value)
configuration = configuration.replace('wi', fan_speed.value)
configuration = configuration.replace('sw', swing_mode.value)
configuration = configuration.replace('tt', hex(int(target_temperature))[2:])

Choose a reason for hiding this comment

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

line too long (85 > 79 characters)

swing_mode: SwingMode):

# Static turn off command available?
if (power == False) and (model in DEVICE_COMMAND_PRESETS) and \

Choose a reason for hiding this comment

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

comparison to False should be 'if cond is False:' or 'if not cond:'

@syssi syssi changed the title WIP: Extended Air Conditioning Companion support Extended Air Conditioning Companion support Jan 21, 2018
if model in DEVICE_COMMAND_TEMPLATES:
configuration = model + DEVICE_COMMAND_TEMPLATES[model]['base']
else:
configuration = model + DEVICE_COMMAND_TEMPLATES['fallback']['base']

Choose a reason for hiding this comment

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

line too long (80 > 79 characters)

else:
configuration = model + DEVICE_COMMAND_TEMPLATES['fallback']['base']

configuration = configuration.replace('po', power.value)
Copy link
Owner

Choose a reason for hiding this comment

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

Just a suggestion, would it be better to replace placeholders with something like [PO] to make it more clear that they are placeholders? Just a thought though, otherwise this seems fine to me so please feel to merge when you want.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point!

},
'0100010727': {
'deviceType': 'gree_2',
'base': '[po][mo][wi][sw][tt]1100190[tt1]205002102000t7t0190[tt1]207002000000[tt4]0',

Choose a reason for hiding this comment

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

line too long (93 > 79 characters)

POWER_OFF = 'off'

# Command templates per model number (f.e. 0180111111)
# [po], [mo], [wi], [sw], [tt], [tt1], [tt4] and [tt7] are markers which will be replaced

Choose a reason for hiding this comment

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

line too long (89 > 79 characters)

@syssi syssi merged commit e9ce9e0 into rytilahti:master Jan 26, 2018
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.

4 participants