Conversation
WalkthroughRemoved device-axis initializations from AYANEOAIRPlus, switched two EC memory reads to a different read function in AYANEODeviceCEc, added rgbConfirmation initialization plus LED group and bypass-charge value/address adjustments in AYANEODeviceCEii, and bumped INNO_VERSION in the publish workflow. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
If "group" == Both, explicitly apply to two sticks. Since the implemention doesn't support set both at once. Fix charge bypass for CEc & CEii. Thanks to @BenjaminLSR Clean up redundant configs in AYANEOAIRPlus.cs
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
HandheldCompanion/Devices/AYANEO/AYANEODeviceCEii.cs (1)
110-126:StickBothhandling in CEii RGB write correctly fans out to left/right without recursion issues.The early
group == LEDGroup.StickBothbranch that delegates toStickLeftandStickRightand then returns is a safe way to support the composite group on CEii while keeping the AYASpace-style address mapping for other groups. There are no recursion or correctness problems here. As a minor readability tweak, you could replace the ternaries forapplyAddress/groupAddresswith a smallswitchover the supported groups to make the mapping more self-documenting, but that’s optional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
HandheldCompanion/Devices/AYANEO/AYANEOAIRPlus.cs(0 hunks)HandheldCompanion/Devices/AYANEO/AYANEODeviceCEc.cs(1 hunks)HandheldCompanion/Devices/AYANEO/AYANEODeviceCEii.cs(3 hunks)
💤 Files with no reviewable changes (1)
- HandheldCompanion/Devices/AYANEO/AYANEOAIRPlus.cs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-07-27T07:08:12.731Z
Learnt from: romracer
Repo: Valkirie/HandheldCompanion PR: 1012
File: HandheldCompanion/Devices/MSI/ClawA1M.cs:0-0
Timestamp: 2024-07-27T07:08:12.731Z
Learning: The `ClawA1M` class in the `HandheldCompanion` project has been refactored to improve readability and maintainability. Changes include the removal of `scope` as a private field, simplification by removing thread logic, encapsulation of WMI event monitoring setup and teardown in separate methods with proper exception handling, and marking `specialKeyWatcher` as nullable.
Applied to files:
HandheldCompanion/Devices/AYANEO/AYANEODeviceCEii.cs
🧬 Code graph analysis (1)
HandheldCompanion/Devices/AYANEO/AYANEODeviceCEii.cs (1)
HandheldCompanion/Devices/AYANEO/AYANEODeviceCEc.cs (2)
CEcControl_RgbI2cWrite(305-313)CEcControl_BypassChargeClose(331-335)
🔇 Additional comments (3)
HandheldCompanion/Devices/AYANEO/AYANEODeviceCEc.cs (1)
325-335: Bypass charge now usesEcReadByteon 0x1e; logic looks consistent but is shared across all CEc devices.The read-before-write pattern against register
0x1ewith values0x55(open) and0xaa(close) is straightforward and matches the existing writes, so the change looks correct from a code perspective. Since this base implementation is used by all CEc-based AYANEO models, please confirm that0x1e/0x55/0xaaare valid for every CEc device you intend to support and that moving from the previous EC RAM read toEcReadBytedoesn’t change semantics on any of them.HandheldCompanion/Devices/AYANEO/AYANEODeviceCEii.cs (2)
64-69: DisablingrgbConfirmationfor CEii to avoid CEc-style confirmation writes makes sense.Setting
this.rgbConfirmation = falseensures the baseCEcRgb_I2cWriteno longer issues the extraStickBothconfirmation command that the CEc path expects, which would otherwise go through your CEii-specificCEcControl_RgbI2cWriteoverride in a non‑AYASpace‑like way. This looks appropriate for the CEii protocol; just confirm that none of the CEii lighting behaviors rely on that extra confirmation pulse.
148-160: CEii bypass-charge overrides on0xd1(0x01open /0x65close) match the intended pattern but rely on firmware-specific values.The overrides keep the same “read EC, only write if different” pattern as the base implementation, now using
ECRamDirectRead/Write(0xd1)with0x01for open and0x65for close, aligned with the AYASpace helper comments. From a code standpoint this is fine; please double-check these magic values and the0xd1address against AYASpace or vendor documentation for all CEii SKUs (AIR Plus variants and Slide) to ensure they behave identically across devices.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/publish-hc.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and Release
| env: | ||
| SOLUTION_NAME: HandheldCompanion | ||
| INNO_VERSION: 6.2.2 | ||
| INNO_VERSION: 6.6.1 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Innosetup 6.6.1 release notes changelog breaking changes
💡 Result:
Short answer: There are no explicit "breaking changes" listed in the Inno Setup 6.6.1 release notes. The release contains a few behavioral/documentation/security changes you may need to check in your scripts:
Notable changes (may affect some setups)
- RTF display with custom styles: default foreground colors are now replaced only when appropriate (previously all foreground colors were replaced by the style foreground). [1][2]
- New [Setup] directive WizardImageOpacity (controls wizard image opacity). Compiler also now stores identical regular and DynamicDark wizard images only once. [1][2]
- Pascal Script doc fix: CreateCustomForm prototype in docs corrected. [1]
- New [Files] section flag notimestamp to avoid storing file timestamps (helps reproducible builds). [1][2]
- Other minor improvements and security fixes. [1]
Sources: Inno Setup 6.6.1 changelog (summaries). [1] [2]
🌐 Web query:
Innosetup version 6.2.2 vs 6.6.1 differences compatibility
💡 Result:
Short summary — what changed between Inno Setup 6.2.2 (Feb 15, 2023) and 6.6.1 (Nov 20, 2025), and compatibility notes
Key differences (high‑level)
- Security / reproducible builds: added [Files] flag notimestamp to avoid storing file timestamps (helps reproducible builds). [1]
- UI / styles: improved custom style handling for RTF foreground colors; new [Setup] directive WizardImageOpacity; identical regular/DynamicDark wizard images are stored only once. Documentation fixes for CreateCustomForm. [1][2]
- Pascal Script additions / fixes across 6.3–6.6: fixes for TStream.Seek, Extract7ZipArchive, improved download/basic-auth support earlier, and various Pascal Script stability/improvements. (Multiple point releases between 6.2.2 → 6.6.1 include script API fixes.) [3][4]
- Compression / toolchain changes: LZMA SDK updated (6.4.x) which affects compression behavior/tuning; other internal tool/Delphi upgrades and minor behavior changes in 6.3–6.5. [5]
- New tooling: ISSigTool.exe (ECDSA P‑256 signatures for verifying some bundled DLLs) introduced as a preview in 6.4.3. [5]
- Miscellaneous: many small IDE/compiler improvements, DPI/font tweaks, high‑quality system icons on some dialogs, CloseApplicationsFilterExcludes, RedirectionGuard logging, and other minor/security/compatibility fixes added in point releases. See full changelogs for details. [3][5][1]
Compatibility implications / migration notes
- Script compatibility: Most .iss scripts remain compatible. However, Pascal Script code can be sensitive to engine changes — review any custom [Code] uses (TStream, Extract7ZipArchive, download APIs, Format behavior) and retest installers after upgrading. If you rely on third‑party tools (e.g., Graphical Installer) confirm compatibility with the target Inno Setup version. [3][4]
- Compression differences: If you rely on exact installer sizes or specific LZMA tuning, re‑test compression settings after 6.4.x (LZMA SDK) because some settings (e.g., LZMABlockSize vs dictionary size) may behave differently. [5]
- Visual/custom style changes: If you use custom styles or custom RTF/wizard images, test them (WizardImageOpacity, color handling, DynamicDark image deduplication). [1]
- Security/behavior under SYSTEM: Various mitigations for DLL preloading and other security fixes were added after 6.2.2 — those can change runtime behavior in restricted contexts; test installers run as SERVICE/SYSTEM where relevant. [3]
Where to read the full details (changelogs)
- 6.6.1 whatsnew (Inno Setup official): jrsoftware.org files/6.6.1 notes. [1]
- 6.4.x and 6.3.x revision histories (notable changes such as LZMA SDK, ISSigTool, Pascal Script fixes): jrsoftware.org whatsnew pages. [5][3]
- 6.2.2 whatsnew (base you’re coming from): jrsoftware.org 6.2.2 notes. [3]
References
- Inno Setup 6.6.1 changelog / whatsnew (jrsoftware). [https://jrsoftware.org/files/is6-whatsnew.htm — see 6.6.1]. [1]
- Inno Setup GitHub mirror / whatsnew (documentation bits). [https://jrsoftware.github.io/issrc/whatsnew.htm]. [2]
- Inno Setup 6.2.2 whatsnew (jrsoftware). [https://jrsoftware.org/files/is6.2-whatsnew.htm]. [3]
- Pascal Script / community notes (examples of fixes and Graphical Installer compatibility discussions). (Inno Setup mailing list / groups). [4]
- Inno Setup 6.4.x revision history (LZMA SDK update, ISSigTool, IDE changes). [https://jrsoftware.org/files/is6.4-whatsnew.htm]. [5]
If you want, I can:
- extract the exact changelog entries for every intermediate version (6.3.0 → 6.6.1), or
- scan your script for likely incompatibilities and list lines to test/adjust. Which would you prefer?
Clarify the necessity of the Innosetup version bump for this PR.
Line 40 updates INNO_VERSION from 6.2.2 to 6.6.1. While 6.6.1 is a stable release with no documented breaking changes, the upgrade spans multiple minor versions (~2.8 years). The PR objectives focus on CEii device fixes (charge bypass, TDP control), and it's unclear why an Innosetup version bump is required or included.
Before merging, clarify:
- Whether this version bump is necessary for the device fixes or is an unrelated update
- Whether your installer scripts or build process depend on changes in 6.6.1
- That the installer still functions correctly with 6.6.1 in your build environment
🤖 Prompt for AI Agents
.github/workflows/publish-hc.yml around line 40: the INNO_VERSION was changed
from 6.2.2 to 6.6.1 without explanation; either revert this bump if it is
unrelated to the CEii device fixes, or provide explicit justification and
verification steps: state in the PR description why 6.6.1 is required for the
fixes (or confirm it is a separate maintenance update), run a full installer
build using 6.6.1 in your CI/local environment and report pass/fail, ensure
installer behavior is unchanged (smoke test install/uninstall), and add a short
note in the workflow commit message or changelog indicating the reason for the
version bump and test results; if the build fails or no dependency exists,
revert to 6.2.2.
|
Inno Setup 6.2.2 doesn't support ArchitecturesInstallIn64BitMode=x64compatible, so I just update that to latest version. |
|
Looks alright. Let me know when you want me to merge. |
Thanks, please merge. Also can you link the bug to close it with merge? Or you'll close it with next release? |
2 participants
Should be a common issue for CEii devices (AIR Plus/Slide). Tested on my AIR Plus AMD.
I'm also trying to fix other potential CEii bugs (charge bypass not working/TDP control not resetting), thus create a darft here.
I'm aiming to finalize this PR before Monday.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.