This repository was archived by the owner on Oct 1, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 15
Bugfixes and new feature -- currently testing at home #40
Draft
jeffmaki
wants to merge
12
commits into
sman591:master
Choose a base branch
from
jeffmaki:jeffmaki-bugfixes
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
a9a041e
Bug fixes and new feature
jeffmaki 9d27afb
Add missing file
jeffmaki e8c7e59
Fix filename problem for case sensitive filesystems
jeffmaki 1645f85
Changes in response to code review
jeffmaki aee8074
ESLint changes (forgot to enable this in VC last commit)
jeffmaki d847daa
Fix bug where eco mode was selected when not available on the unit, e…
jeffmaki 055a511
Apply suggestions from code review
jeffmaki 036e2a0
lint fix after code review
jeffmaki c4270e7
One more fix after code review for lint
jeffmaki 304e6da
Update platformAccessory.ts
jeffmaki 825261e
Changes from code review
jeffmaki 40450ac
Remove duplicate call
jeffmaki File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,26 +4,32 @@ import type { | |
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| Characteristic as HomebridgeCharacteristic, | ||
| CharacteristicValue, | ||
| CharacteristicChange, | ||
| CharacteristicGetCallback, | ||
| CharacteristicSetCallback, | ||
| WithUUID, | ||
| } from 'homebridge' | ||
|
|
||
| import { HomebridgeLgThinqPlatform } from '../platform' | ||
| import type { LgAirConditionerPlatformAccessory } from '../platformAccessory' | ||
| import type { GetDeviceResponse } from '../thinq/apiTypes' | ||
|
|
||
| type Unpacked<T> = T extends (infer U)[] ? U : T | ||
|
|
||
| export default abstract class AbstractCharacteristic< | ||
| State extends CharacteristicValue, | ||
| ApiValue extends string | number, | ||
| Characteristic extends WithUUID<{ | ||
| new (): HomebridgeCharacteristic | ||
| }> /** Comes from this.platform.Characteristic.____ */ | ||
| > { | ||
| private platform: HomebridgeLgThinqPlatform | ||
| private service: Service | ||
| private deviceId: string | ||
| protected platform: HomebridgeLgThinqPlatform | ||
| protected service: Service | ||
| protected device: LgAirConditionerPlatformAccessory | ||
| protected characteristic: Characteristic /** Comes from this.platform.Characteristic.____ */ | ||
|
|
||
| private cachedState?: State | ||
| characteristic: Characteristic /** Comes from this.platform.Characteristic.____ */ | ||
|
|
||
| private apiCommand: 'Set' | 'Operation' | ||
| private apiDataKey: keyof GetDeviceResponse['result']['snapshot'] | ||
|
|
||
|
|
@@ -34,14 +40,14 @@ export default abstract class AbstractCharacteristic< | |
| constructor( | ||
| platform: HomebridgeLgThinqPlatform, | ||
| service: Service, | ||
| deviceId: string, | ||
| device: LgAirConditionerPlatformAccessory, | ||
| characteristic: Characteristic, | ||
| apiCommand: 'Set' | 'Operation', | ||
| apiDataKey: keyof GetDeviceResponse['result']['snapshot'], | ||
| ) { | ||
| this.platform = platform | ||
| this.service = service | ||
| this.deviceId = deviceId | ||
| this.device = device | ||
| this.characteristic = characteristic | ||
| this.apiCommand = apiCommand | ||
| this.apiDataKey = apiDataKey | ||
|
|
@@ -56,6 +62,12 @@ export default abstract class AbstractCharacteristic< | |
| .getCharacteristic(this.characteristic) | ||
| .on(CharacteristicEventTypes.SET, this.handleSet.bind(this)) | ||
| } | ||
|
|
||
| if (this.handleChange) { | ||
| this.service | ||
| .getCharacteristic(this.characteristic) | ||
| .on(CharacteristicEventTypes.CHANGE, this.handleChange.bind(this)) | ||
| } | ||
| } | ||
|
|
||
| /** Transform Homebridge state to what the ThinQ API expects */ | ||
|
|
@@ -67,20 +79,34 @@ export default abstract class AbstractCharacteristic< | |
| abstract getApiValueFromState(state: State): ApiValue | ||
|
|
||
| /** Take in an updated device snapshot */ | ||
| handleUpdatedSnapshot(snapshot: GetDeviceResponse['result']['snapshot']) { | ||
| handleUpdatedSnapshot( | ||
| snapshot: Unpacked<GetDeviceResponse['result']['snapshot']>, | ||
| ) { | ||
| try { | ||
| this.logDebug('HandleSnapshot for ' + this.characteristic.name) | ||
|
|
||
| const apiValue = snapshot[this.apiDataKey] as ApiValue | ||
| this.logDebug('handleUpdatedSnapshot', apiValue) | ||
|
|
||
| this.cachedState = this.getStateFromApiValue(apiValue) | ||
| this.service.updateCharacteristic(this.characteristic, this.cachedState) | ||
| } catch (error) { | ||
| this.logError('Error parsing state', error.toString()) | ||
| } | ||
| } | ||
|
|
||
| /** Handle a "change" command from Homebridge to update this characteristic */ | ||
| handleChange?(value: CharacteristicChange) { | ||
| this.logDebug( | ||
| 'Triggered CHANGE ignored! Consider implementing for this characteristic. newValue:', | ||
| value.newValue, | ||
| ) | ||
| } | ||
|
|
||
| /** Handle a "set" command from Homebridge to update this characteristic */ | ||
| handleSet?(value: CharacteristicValue, callback: CharacteristicSetCallback) { | ||
| this.logDebug('Triggered SET:', value) | ||
jeffmaki marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| if (!this.thinqApi) { | ||
| this.logError('API not initialized yet') | ||
| return | ||
|
|
@@ -92,9 +118,9 @@ export default abstract class AbstractCharacteristic< | |
| ) | ||
| this.logDebug('targetState', targetState) | ||
|
|
||
| // The air conditioner will make a sound every time this API is called. | ||
| // To avoid unnecessary chimes, we'll optimistically skip sending the API call. | ||
| if (targetState === this.cachedState) { | ||
| // The air conditioner will make a sound every time this API is called. | ||
| // To avoid unnecessary chimes, we'll optimistically skip sending the API call. | ||
| this.logDebug('State equals cached state. Skipping.', targetState) | ||
| callback(null, targetState) | ||
| return | ||
|
|
@@ -103,7 +129,12 @@ export default abstract class AbstractCharacteristic< | |
| const apiValue = this.getApiValueFromState(targetState) | ||
|
|
||
| this.thinqApi | ||
| .sendCommand(this.deviceId, this.apiCommand, this.apiDataKey, apiValue) | ||
| .sendCommand( | ||
| this.device.getDeviceId() || '', | ||
| this.apiCommand, | ||
| this.apiDataKey, | ||
| apiValue, | ||
| ) | ||
| .then(() => { | ||
| this.cachedState = targetState | ||
| callback(null, targetState) | ||
|
|
@@ -134,4 +165,91 @@ export default abstract class AbstractCharacteristic< | |
| ...parameters, | ||
| ) | ||
| } | ||
|
|
||
| deviceUsesFahrenheit(): boolean { | ||
| if (this.device.getDevice()) { | ||
| return this.device.getDevice()!.countryCode.startsWith('US') | ||
| } else { | ||
| return false | ||
| } | ||
| } | ||
|
|
||
| roundHalf(r: number): number { | ||
| return Math.round(r * 2) / 2 | ||
| } | ||
|
|
||
| /* | ||
| LG Air Conditioners use a lookup table to go between Celsius and Farenheit, which we request from the | ||
| LG servers as part of the GetDashboard call. There are two tables, CelToFah and FahToCel, which are *not* | ||
| inverses of each other (unfortunately). | ||
|
|
||
| These tables are the same as the units use, so if you want your results to match what they physically display, | ||
| you need to use the lookup tables. | ||
|
|
||
| HomeKit's API uses Celsius as its standard unit, regardless of whether the user sees Farenheit on the UI or not--that is, | ||
| conversion happens on the iPhone. | ||
|
|
||
| Thus, to ensure the temperatures on the app and the unit match, we have to convert to Farenheit, | ||
| then back to Celsius for HomeKit, the latter conversion using the classic (5/9) - 32 math. | ||
|
|
||
| We do all this only if the user sees Farenheit, as I *think* this issue is moot if the units show celsius? A non-US user to confirm. | ||
| */ | ||
| getHomeKitCelsiusForLGAPICelsius(_celsius: number): number { | ||
| if (!this.deviceUsesFahrenheit()) { | ||
| return _celsius | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there harm in always performing the conversion? I believe both Celsius and Fahrenheit users will see these discrepancies due to rounding differences, right?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above |
||
| } | ||
|
|
||
| const LGCelsius = this.roundHalf(_celsius) | ||
| const LGCelsiusToF: Partial<Record< | ||
| string, | ||
| number | ||
| >> = this.device.getModelInfo().Value.TempCelToFah.value_mapping | ||
| const LGFarenheit = LGCelsiusToF[LGCelsius] | ||
|
|
||
| if (LGFarenheit === undefined) { | ||
| this.logError( | ||
| 'getHomeKitCelsiusForLGAPICelsius input temperature ' + | ||
| _celsius + | ||
| ' was not found in LG mapping table', | ||
| ) | ||
| return _celsius | ||
| } | ||
|
|
||
| const HKCelsius = this.roundHalf((LGFarenheit - 32) * (5 / 9)) | ||
| this.logDebug( | ||
| 'getHomeKitCelsiusForLGAPICelsius in=' + _celsius + ' out=' + HKCelsius, | ||
| ) | ||
| return HKCelsius | ||
| } | ||
|
|
||
| // inverse of the above | ||
| getLGAPICelsiusForHomeKitCelsius(_celsius: number): number { | ||
| if (!this.deviceUsesFahrenheit()) { | ||
| return _celsius | ||
| } | ||
|
|
||
| const HKCelsiusInFarenheit: number = Math.round(_celsius * (9 / 5) + 32) | ||
| const LGCelsiusToF = this.device.getModelInfo().Value.TempCelToFah | ||
| .value_mapping | ||
|
|
||
| for (const LGCelsius in LGCelsiusToF) { | ||
| const LGFarenheit = LGCelsiusToF[LGCelsius] | ||
|
|
||
| if (LGFarenheit === HKCelsiusInFarenheit) { | ||
| this.logDebug( | ||
| 'getLGAPICelsiusForHomeKitCelsius in=' + | ||
| _celsius + | ||
| ' out=' + | ||
| LGCelsius, | ||
| ) | ||
|
|
||
| return Number(LGCelsius) | ||
| } | ||
| } | ||
|
|
||
| this.logError( | ||
| 'Value ' + _celsius + " wasn't found in the LG mapping table.", | ||
| ) | ||
| return _celsius | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.