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

AdaptiveLightingController fix & improvement #1038

Conversation

Shaquu
Copy link
Contributor

@Shaquu Shaquu commented Jun 22, 2024

♻️ Current situation

Describe the current situation. Explain current problems, if there are any. Be as descriptive as possible (e.g., including examples or code snippets).

Would be great to have update data provided from update event.
Also noticed few type issues, now properly using optional chaining.

💡 Proposed solution

Describe the proposed solution and changes. How does it affect the project? How does it affect the internal structure (e.g., refactorings)?

Would be great to have update data provided from update event.
Also noticed few type issues, now properly using optional chaining.

⚙️ Release Notes

Provide a summary of the changes or features from a user's point of view. If there are breaking changes, provide migration guides using code examples of the affected features.

AdaptiveLightingController: Update payload added to update event.
AdaptiveLightingController: Resolve possible TypeError, related to incorrect optional values unwrapping.

➕ Additional Information

If applicable, provide additional context in this section.

Testing

Which tests were added? Which existing tests were adapted/changed? Which situations are covered, and what edge cases are missing?

N/A

Reviewer Nudging

Where should the reviewer start? what is a good entry point?

Adaptive Lightning API :)

donavanbecker
donavanbecker previously approved these changes Jun 23, 2024
@Shaquu
Copy link
Contributor Author

Shaquu commented Jun 23, 2024

No clue why some checks failed.
typedoc complains about @since iOS 14 which is not scope of my change
pr labeler cannot start at all

donavanbecker
donavanbecker previously approved these changes Jun 24, 2024
@donavanbecker donavanbecker changed the base branch from latest to beta-0.12.3 June 24, 2024 15:00
@donavanbecker donavanbecker dismissed their stale review June 24, 2024 15:00

The base branch was changed.

@github-actions github-actions bot added the beta This is in some form related to the current beta release label Jun 24, 2024
@donavanbecker donavanbecker changed the base branch from beta-0.12.3 to latest June 24, 2024 15:02
@donavanbecker donavanbecker self-requested a review June 24, 2024 15:02
donavanbecker
donavanbecker previously approved these changes Jun 24, 2024
@donavanbecker donavanbecker requested a review from bwp91 June 24, 2024 15:02
@donavanbecker donavanbecker mentioned this pull request Jun 24, 2024
@Shaquu Shaquu changed the base branch from latest to beta-0.12.3 June 24, 2024 15:22
@Shaquu Shaquu dismissed donavanbecker’s stale review June 24, 2024 15:22

The base branch was changed.

@Shaquu
Copy link
Contributor Author

Shaquu commented Jun 24, 2024

Ahh @donavanbecker I see I did the same as you. Not sure if good. Should it target latest or beta?

@donavanbecker
Copy link
Contributor

Ahh @donavanbecker I see I did the same as you. Not sure if good. Should it target latest or beta?

lets do beta. We can then do a quick Homebridge beta before release this version.

@donavanbecker donavanbecker enabled auto-merge (squash) June 24, 2024 15:35
@bwp91
Copy link
Contributor

bwp91 commented Jun 24, 2024

  • this won't cause any issues with existing plugins that use adaptive lighting API right? (just wanted to confirm)
  • it would be nice if you could update src/accessories/Light-AdaptiveLighting_accessory.ts with perhaps just a simple way of showing how the new update information can be retrieved

@Shaquu
Copy link
Contributor Author

Shaquu commented Jun 24, 2024

Good point. Let me check for both, let's not rush it.

@bwp91
Copy link
Contributor

bwp91 commented Jun 24, 2024

also if you update your branch from the beta branch, you should find the build issues are all fixed 👍

@coveralls
Copy link

coveralls commented Jun 24, 2024

Pull Request Test Coverage Report for Build 9649422541

Details

  • 0 of 17 (0.0%) changed or added relevant lines in 1 file are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.3%) to 64.785%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/lib/controller/AdaptiveLightingController.ts 0 17 0.0%
Files with Coverage Reduction New Missed Lines %
src/lib/Accessory.ts 3 89.29%
Totals Coverage Status
Change from base Build 9649015627: -0.3%
Covered Lines: 7411
Relevant Lines: 10616

💛 - Coveralls

auto-merge was automatically disabled June 24, 2024 21:48

Head branch was pushed to by a user without write access

@coveralls
Copy link

coveralls commented Jun 24, 2024

Pull Request Test Coverage Report for Build 9652898208

Details

  • 0 of 17 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.2%) to 64.855%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/lib/controller/AdaptiveLightingController.ts 0 17 0.0%
Totals Coverage Status
Change from base Build 9649015627: -0.2%
Covered Lines: 7414
Relevant Lines: 10616

💛 - Coveralls

@Shaquu
Copy link
Contributor Author

Shaquu commented Jun 24, 2024

@bwp91

  • this won't cause any issues with existing plugins that use adaptive lighting API right? (just wanted to confirm)

With the new parameter in listener, you can still use listener without a parameter. So current implementations should be all right.

  • it would be nice if you could update src/accessories/Light-AdaptiveLighting_accessory.ts with perhaps just a simple way of showing how the new update information can be retrieved

Updated, but as AUTOMATIC is default I put some comments and some code.

@Shaquu
Copy link
Contributor Author

Shaquu commented Jun 24, 2024

Anyone, PR Labeler / stale / update_release_draft (pull_request) failing
image

@coveralls
Copy link

coveralls commented Jun 24, 2024

Pull Request Test Coverage Report for Build 9652969910

Details

  • 0 of 17 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.2%) to 64.855%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/lib/controller/AdaptiveLightingController.ts 0 17 0.0%
Totals Coverage Status
Change from base Build 9652967305: -0.2%
Covered Lines: 7414
Relevant Lines: 10616

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 24, 2024

Pull Request Test Coverage Report for Build 9653108648

Details

  • 0 of 17 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.2%) to 64.855%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/lib/controller/AdaptiveLightingController.ts 0 17 0.0%
Totals Coverage Status
Change from base Build 9652967305: -0.2%
Covered Lines: 7414
Relevant Lines: 10616

💛 - Coveralls

@donavanbecker donavanbecker merged commit a99b99f into homebridge:beta-0.12.3 Jun 25, 2024
14 checks passed
donavanbecker added a commit that referenced this pull request Jun 26, 2024
* AdaptiveLightingController on update should provide some data without the need to build it yourself

* Fix TypeError

* Fixed linting and test issues

* Typedoc fixes

* Updated examples

* Eslint fixes

---------

Co-authored-by: Donavan Becker <[email protected]>
donavanbecker added a commit that referenced this pull request Jun 28, 2024
* Update pr-labeler.yml

* AdaptiveLightingController fix & improvement (#1038)

* AdaptiveLightingController on update should provide some data without the need to build it yourself

* Fix TypeError

* Fixed linting and test issues

* Typedoc fixes

* Updated examples

* Eslint fixes

---------

Co-authored-by: Donavan Becker <[email protected]>

* BridgedCore.ts and Core.ts cleanup (updated deprecation date from year 2022 to 2024).

* Removed not allowed char from Sprinkler accessory example

* Code review remarks

---------

Co-authored-by: Donavan Becker <[email protected]>
donavanbecker added a commit that referenced this pull request Jul 11, 2024
## v1.0.0 (2025-07-11)

### Breaking Changes

- **The minimum Node.js version required is now `v18`.**
- **Important notice:** Because of the cleanup of the Deprecated code, you will need to migrate you code base.
    - Remove the long-deprecated init().
    - Deprecate Core, BridgedCore, legacy Camera characteristics. (#1058) (@hjdhjd)
        - For deprecated `Core` and `BridgedCore` see: https://github.com/homebridge/HAP-NodeJS/wiki/Deprecation-of-Core-and-BridgeCore
    - Legacy code deprecation cleanup. (#1059) (@hjdhjd)
        - For deprecated `storagePath` switch to `HAPStorage.setCustomStoragePath`, `AudioCodec` switch to `AudioStreamingCodec`, `VideoCodec` switch to `H264CodecParameters`,`StreamAudioParams` switch to `AudioStreamingOptions`, `StreamVideoParams` switch to `VideoStreamingOptions`,`cameraSource` switch to `CameraController`.
    - Others deprecated code to highlight removed: `useLegacyAdvertiser`, `AccessoryLoader`,

### Fixed

- Fix: Build Issues (#1041) (@NorthernMan54)
- Fix: Ensure data is only transmitted on open and ready connections. (#1051) (@hjdhjd)
- Fix: Ensure we check names using the full UTF-8 character set. (#1052) (@hjdhjd)
- Fix: ConfiguredName (#1049) (@donavanbecker)
- Fix: Manufacturer looking at checkName but should look at checkValue. (#1053) (@donavanbecker)

### Other Changes

- Implement warning messages for invalid characters in names (#1009) (@NorthernMan54)
- Mitigate event emitter "memory leak" warnings when a significant number of camera streaming events occur simultaneously (#1037) (@hjdhjd)
- AdaptiveLightingController fix & improvement (#1038) (@Shaquu)
- Minor fixes to recording logging and one change in logging. (#1040) (@hjdhjd)
- Bridged core and core cleanup (#1048) (@Shaquu)
- Increase snapshot handler warning timeout to 8000ms. (#1055) (@hjdhjd)
- Cleanup and refactor getLocalNetworkInterface and address a potential edge case. (#1056) (@hjdhjd)
- Correct log spacing
- Updated and fixed `typedoc` config file
- Updated dependencies

### Homebridge Dependencies

- `@homebridge/ciao` @ `v1.3.0`
- `bonjour-hap` @ `v3.8.0`
donavanbecker added a commit that referenced this pull request Jul 11, 2024
- **The minimum Node.js version required is now `v18`.**
- **Important notice:** Because of the cleanup of the Deprecated code, you will need to migrate you code base.
    - Remove the long-deprecated init().
    - Deprecate Core, BridgedCore, legacy Camera characteristics. (#1058) (@hjdhjd)
        - For deprecated `Core` and `BridgedCore` see: https://github.com/homebridge/HAP-NodeJS/wiki/Deprecation-of-Core-and-BridgeCore
    - Legacy code deprecation cleanup. (#1059) (@hjdhjd)
        - For deprecated `storagePath` switch to `HAPStorage.setCustomStoragePath`, `AudioCodec` switch to `AudioStreamingCodec`, `VideoCodec` switch to `H264CodecParameters`,`StreamAudioParams` switch to `AudioStreamingOptions`, `StreamVideoParams` switch to `VideoStreamingOptions`,`cameraSource` switch to `CameraController`.
    - Others deprecated code to highlight removed: `useLegacyAdvertiser`, `AccessoryLoader`.
- Fix: Naming for Characteristic.ProgramMode has been corrected from `PROGRAM_SCHEDULED_MANUAL_MODE_` to `PROGRAM_SCHEDULED_MANUAL_MODE`

- Fix: Build Issues (#1041) (@NorthernMan54)
- Fix: Ensure data is only transmitted on open and ready connections. (#1051) (@hjdhjd)
- Fix: Ensure we check names using the full UTF-8 character set. (#1052) (@hjdhjd)
- Fix: ConfiguredName (#1049) (@donavanbecker)
- Fix: Manufacturer looking at checkName but should look at checkValue. (#1053) (@donavanbecker)

- Implement warning messages for invalid characters in names (#1009) (@NorthernMan54)
- Mitigate event emitter "memory leak" warnings when a significant number of camera streaming events occur simultaneously (#1037) (@hjdhjd)
- AdaptiveLightingController fix & improvement (#1038) (@Shaquu)
- Minor fixes to recording logging and one change in logging. (#1040) (@hjdhjd)
- Bridged core and core cleanup (#1048) (@Shaquu)
- Increase snapshot handler warning timeout to 8000ms. (#1055) (@hjdhjd)
- Cleanup and refactor getLocalNetworkInterface and address a potential edge case. (#1056) (@hjdhjd)
- Correct log spacing
- Updated and fixed `typedoc` config file
- Updated dependencies

- `@homebridge/ciao` @ `v1.3.0`
- `bonjour-hap` @ `v3.8.0`
donavanbecker added a commit that referenced this pull request Jul 11, 2024
## v1.0.0 (2024-07-10)

### Breaking Changes

- **The minimum Node.js version required is now `v18`.**
- **Important notice:** Because of the cleanup of the Deprecated code,
you will need to migrate you code base.
    - Remove the long-deprecated init().
- Deprecate Core, BridgedCore, legacy Camera characteristics. (#1058)
(@hjdhjd)
- For deprecated `Core` and `BridgedCore` see:
https://github.com/homebridge/HAP-NodeJS/wiki/Deprecation-of-Core-and-BridgeCore
    - Legacy code deprecation cleanup. (#1059) (@hjdhjd)
- For deprecated `storagePath` switch to
`HAPStorage.setCustomStoragePath`, `AudioCodec` switch to
`AudioStreamingCodec`, `VideoCodec` switch to
`H264CodecParameters`,`StreamAudioParams` switch to
`AudioStreamingOptions`, `StreamVideoParams` switch to
`VideoStreamingOptions`,`cameraSource` switch to `CameraController`.
- Others deprecated code to highlight removed: `useLegacyAdvertiser`,
`AccessoryLoader`.
- Fix: Naming for Characteristic.ProgramMode has been corrected from
`PROGRAM_SCHEDULED_MANUAL_MODE_` to `PROGRAM_SCHEDULED_MANUAL_MODE`

### Fixed

- Fix: Build Issues (#1041) (@NorthernMan54)
- Fix: Ensure data is only transmitted on open and ready connections.
(#1051) (@hjdhjd)
- Fix: Ensure we check names using the full UTF-8 character set. (#1052)
(@hjdhjd)
- Fix: ConfiguredName (#1049) (@donavanbecker)
- Fix: Manufacturer looking at checkName but should look at checkValue.
(#1053) (@donavanbecker)

### Other Changes

- Implement warning messages for invalid characters in names (#1009)
(@NorthernMan54)
- Mitigate event emitter "memory leak" warnings when a significant
number of camera streaming events occur simultaneously (#1037) (@hjdhjd)
- AdaptiveLightingController fix & improvement (#1038) (@Shaquu)
- Minor fixes to recording logging and one change in logging. (#1040)
(@hjdhjd)
- Bridged core and core cleanup (#1048) (@Shaquu)
- Increase snapshot handler warning timeout to 8000ms. (#1055) (@hjdhjd)
- Cleanup and refactor getLocalNetworkInterface and address a potential
edge case. (#1056) (@hjdhjd)
- Correct log spacing
- Updated and fixed `typedoc` config file
- Updated dependencies

### Homebridge Dependencies

- `@homebridge/ciao` @ `v1.3.0`
- `bonjour-hap` @ `v3.8.0`
@bwp91 bwp91 mentioned this pull request Jul 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta This is in some form related to the current beta release feature latest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants