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

add more function for hvac #36

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

wkd8176
Copy link

@wkd8176 wkd8176 commented Jul 9, 2019

LG AC has 3 kind model roughly, RAC, PAC, SAC
SAC is system airconditioner(mostly for commercial), PAC is stand type model, RAC is wall mount type model at least in Korea.
So I added these feature below.

  1. Add more ac mode(for PAC, RAC, SAC)
  2. Add more fan speed(for PAC, RAC, SAC)
  3. Add swing mode
  4. Add reserve mode
  5. Add Extra mode (may be some mode doesn't use at all)
  6. Add RAC Submode
  7. Add air polution mode
  8. Add airclean mode
  9. Add wind direction mode(left/right, step)
  10. get power usage data
  11. add support device function -> In same model type, support different functions each device . so i have to find what function support each device.

LG AC has 3 kind model roughly, RAC, PAC, SAC
SAC is system airconditioner(mostly for commercial), PAC is stand type model, RAC is wall mount type model at least in Korea.
So I added these feature below.
1. Add more ac mode(for PAC, RAC, SAC)
2. Add more fan speed(for PAC, RAC, SAC)
3. Add swing mode
4. Add reserve mode
5. Add Extra mode (may be some mode doesn't use at all)
6. Add RAC Submode
7. Add air polution mode
8. Add airclean mode
9. Add wind direction mode(left/right, step)
10. get power usage data
11. add support device function -> In same model type, support different functions each device . so i have to find what function support each device.

class ACDevice(Device):
"""Higher-level operations on an AC/HVAC device, such as a heat
pump.
"""

Copy link
Owner

Choose a reason for hiding this comment

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

Looks like more inadvertent whitespace changes.

TIMEBS = "@TIMEBS_ONOFF"
WEEKLYSCHEDULE = "@WEEKLY_SCHEDULE"

class ACEXTRAMode(enum.Enum):
Copy link
Owner

Choose a reason for hiding this comment

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

Class names should not use all caps.

mode_value = self.model.enum_value('AirClean', mode.value)
self._set_control('AirClean', mode_value)

def set_etc_mode(self, name, is_on):
Copy link
Owner

Choose a reason for hiding this comment

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

What's the difference between "etc mode" and "extra mode"?

"""Get outdoor weather"""

data = self.client.session.get_outdoor_weather(area)
return data
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think this is necessary here—the API is associated with the session, not the device.

time_data = value[i].split('_')
time.append(int(time_data[1]))
i = i+1
time_sum = sum(time)
Copy link
Owner

Choose a reason for hiding this comment

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

This stuff looks copied & pasted—can we move it to its own helper function?

dict_support_airpolution = self.ac.model.option_item('SupportAirPolution')
support_airpolution = []
for option in dict_support_airpolution.values():
support_airpolution.append(ACAirPolution(option).name)
Copy link
Owner

Choose a reason for hiding this comment

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

These "support" functions also share a very common structure. Maybe they can be factored out into a single helper function. Using a list comprehension could also make them more readable (and shorter).


@property
def sensorpm10(self):
return self.data['SensorPM10']
Copy link
Owner

Choose a reason for hiding this comment

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

What do these "sensorpmX" properties mean?

If possible, I would love to only expose things in this API that we actually understand. The LG service has a lot of inscrutable garbage in it that is probably not necessary for any client software, and we can keep things simpler by omitting that stuff.

Choose a reason for hiding this comment

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

My guess is those values are PM2.5 and PM10 particulate matter, mostly as a smog.
https://en.wikipedia.org/wiki/Particulates

@dacrypt
Copy link
Contributor

dacrypt commented Jan 21, 2020

One of many promising PRs that never were accepted. What a pity

@sampsyo
Copy link
Owner

sampsyo commented Jan 21, 2020

Hi, @dacrypt—if you're interested in seeing this merged, maybe you can help out! I left a review with lots of opportunities to improve the code.

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.

5 participants