Added friendly product names for well-known hardware IDs#1
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a USB VID/PID → friendly-name lookup and surfaces per-slot display names from input backends into the UI. Backends expose GetSlotDisplayName() (and GetName()), main collects display names per slot, and the gamepad renderer shows them in the panel header when present. Changes
Sequence Diagram(s)sequenceDiagram
participant Main as Main (collector)
participant Backend as InputBackend (dinput/hidapi/rawinput)
participant Names as usb_names
participant Renderer as GamepadRenderer
Main->>Backend: GetSlotDisplayName(slot)
Backend->>Names: GetFriendlyName(vid, pid)
Names-->>Backend: friendlyName or nullptr
Backend-->>Main: displayName or nullptr
Main->>Renderer: DrawGamepad(..., backendName, displayName)
Renderer-->>Main: rendered UI panel with header (Player N [- displayName] [backend])
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/input_backend.h (1)
19-19: Avoid unused-parameter warnings in the default method.Line 19 names
slotbut does not use it; strict warning settings can flag this. Consider marking it unused in the signature.♻️ Proposed tweak
- [[nodiscard]] virtual const char* GetSlotDisplayName(int slot) const { return nullptr; } + [[nodiscard]] virtual const char* GetSlotDisplayName(int /*slot*/) const { return nullptr; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/input_backend.h` at line 19, The default virtual method GetSlotDisplayName(int slot) names the parameter but doesn't use it, causing unused-parameter warnings; update the declaration/definition of GetSlotDisplayName to mark the parameter as unused (for example remove the parameter name, use [[maybe_unused]] int slot, or omit the name in the signature) so the compiler won't warn while preserving the method signature and semantics.src/gamepad_renderer.cpp (1)
161-171: Consider clipping the header to avoid overlap with the status pill.With long friendly names (Line 163), header text can collide with the right-side status indicator. A clipped header draw region would keep layout stable.
✂️ Possible UI-safe adjustment
- ImVec2 hts = ImGui::CalcTextSize(header.c_str()); + const char* status = gs.connected ? "Connected" : "Not Connected"; + ImVec2 hts = ImGui::CalcTextSize(header.c_str()); float headerH = hts.y + 10.0f; - dl->AddText(ImVec2(panelPos.x + 10, panelPos.y + 5), - gs.connected ? IM_COL32(220, 220, 220, 255) - : IM_COL32(100, 100, 100, 255), - header.c_str()); + float statusReserve = ImGui::CalcTextSize(status).x + 26.0f; + float headerRight = panelPos.x + panelSize.x - statusReserve; + dl->PushClipRect(ImVec2(panelPos.x + 10, panelPos.y), + ImVec2(headerRight, panelPos.y + headerH), true); + dl->AddText(ImVec2(panelPos.x + 10, panelPos.y + 5), + gs.connected ? IM_COL32(220, 220, 220, 255) + : IM_COL32(100, 100, 100, 255), + header.c_str()); + dl->PopClipRect();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gamepad_renderer.cpp` around lines 161 - 171, The header string can overlap the right-side status pill when displayName is long; compute the available width between panelPos.x+10 and the status pill X (or panelPos.x + panelWidth - rightPadding), then either clamp/truncate header or restrict drawing to that rectangle using ImGui::PushClipRect/PopClipRect (or use ImGui::CalcTextSize with a wrap/clip width) before calling dl->AddText with header.c_str(); adjust header creation (involving header, slotIndex, displayName, backendName) so the drawn text never extends into the status pill area and restore clipping after drawing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/gamepad_renderer.cpp`:
- Around line 161-171: The header string can overlap the right-side status pill
when displayName is long; compute the available width between panelPos.x+10 and
the status pill X (or panelPos.x + panelWidth - rightPadding), then either
clamp/truncate header or restrict drawing to that rectangle using
ImGui::PushClipRect/PopClipRect (or use ImGui::CalcTextSize with a wrap/clip
width) before calling dl->AddText with header.c_str(); adjust header creation
(involving header, slotIndex, displayName, backendName) so the drawn text never
extends into the status pill area and restore clipping after drawing.
In `@src/input_backend.h`:
- Line 19: The default virtual method GetSlotDisplayName(int slot) names the
parameter but doesn't use it, causing unused-parameter warnings; update the
declaration/definition of GetSlotDisplayName to mark the parameter as unused
(for example remove the parameter name, use [[maybe_unused]] int slot, or omit
the name in the signature) so the compiler won't warn while preserving the
method signature and semantics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 701f5b82-2e84-4499-b8ce-968eef5ca272
📒 Files selected for processing (12)
src/dinput_backend.cppsrc/dinput_backend.hsrc/gamepad_renderer.cppsrc/gamepad_renderer.hsrc/hidapi_backend.cppsrc/hidapi_backend.hsrc/input_backend.hsrc/main.cppsrc/rawinput_backend.cppsrc/rawinput_backend.hsrc/usb_names.cppsrc/usb_names.h
Contributor
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @nefarius. The following files were modified: * `src/dinput_backend.cpp` * `src/dinput_backend.h` * `src/gamepad_renderer.cpp` * `src/gamepad_renderer.h` * `src/hidapi_backend.cpp` * `src/hidapi_backend.h` * `src/input_backend.h` * `src/main.cpp` * `src/rawinput_backend.cpp` * `src/rawinput_backend.h` * `src/usb_names.cpp`
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/rawinput_backend.cpp (1)
87-90:⚠️ Potential issue | 🟡 Minor
PPimplementation does not match its documented null-on-empty contract.The docs (Line 84) say this returns
nullptrwhen the preparsed buffer is empty, but the implementation always castsdata(). Please either guard empties or update the docs.🔧 Proposed fix
PHIDP_PREPARSED_DATA RawInputBackend::PP(DeviceInfo& d) { - return reinterpret_cast<PHIDP_PREPARSED_DATA>(d.preparsedBuf.data()); + if (d.preparsedBuf.empty()) return nullptr; + return reinterpret_cast<PHIDP_PREPARSED_DATA>(d.preparsedBuf.data()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rawinput_backend.cpp` around lines 87 - 90, The PP function currently always casts d.preparsedBuf.data() but must honor the documented contract of returning nullptr when the preparsed buffer is empty; modify RawInputBackend::PP to check d.preparsedBuf.empty() (or size()==0) and return nullptr in that case, otherwise return reinterpret_cast<PHIDP_PREPARSED_DATA>(d.preparsedBuf.data()); reference the PP method and DeviceInfo::preparsedBuf when making this change.
🧹 Nitpick comments (1)
src/dinput_backend.h (1)
41-44: Consider documenting nullability explicitly for consistency.Optional: state that
GetSlotDisplayNamecan returnnullptrwhen no slot/device mapping exists, matching the interface/base contract style.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/dinput_backend.h` around lines 41 - 44, Update the comment for GetSlotDisplayName to explicitly document nullability: state that the function returns a pointer to a null-terminated string describing the slot or its assigned device and that it may return nullptr when no slot/device mapping exists (or on absence of a name). Modify the docblock for GetSlotDisplayName to include the "May return nullptr" wording so it matches the interface/base contract style used elsewhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/hidapi_backend.h`:
- Around line 42-45: Update the docblock for GetSlotDisplayName to match its
implementation: state that it returns a null-terminated display name for the
slot or nullptr when no name is available (instead of an empty string).
Reference the GetSlotDisplayName function and ensure the comment's return
description aligns with the behavior in src/hidapi_backend.cpp where nullptr is
returned for unavailable names.
In `@src/rawinput_backend.h`:
- Around line 45-50: Update the GetSlotDisplayName documentation to match its
implementation: state that it returns a null-terminated string on success and
returns nullptr when no display name is available (rather than an empty string).
Modify the comment for GetSlotDisplayName to mention the nullptr return contract
and clarify ownership/lifetime semantics of the returned pointer to match the
behavior in the GetSlotDisplayName implementation.
---
Outside diff comments:
In `@src/rawinput_backend.cpp`:
- Around line 87-90: The PP function currently always casts
d.preparsedBuf.data() but must honor the documented contract of returning
nullptr when the preparsed buffer is empty; modify RawInputBackend::PP to check
d.preparsedBuf.empty() (or size()==0) and return nullptr in that case, otherwise
return reinterpret_cast<PHIDP_PREPARSED_DATA>(d.preparsedBuf.data()); reference
the PP method and DeviceInfo::preparsedBuf when making this change.
---
Nitpick comments:
In `@src/dinput_backend.h`:
- Around line 41-44: Update the comment for GetSlotDisplayName to explicitly
document nullability: state that the function returns a pointer to a
null-terminated string describing the slot or its assigned device and that it
may return nullptr when no slot/device mapping exists (or on absence of a name).
Modify the docblock for GetSlotDisplayName to include the "May return nullptr"
wording so it matches the interface/base contract style used elsewhere.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b6a80016-71a9-449d-a123-fc25a2f8761d
📒 Files selected for processing (11)
src/dinput_backend.cppsrc/dinput_backend.hsrc/gamepad_renderer.cppsrc/gamepad_renderer.hsrc/hidapi_backend.cppsrc/hidapi_backend.hsrc/input_backend.hsrc/main.cppsrc/rawinput_backend.cppsrc/rawinput_backend.hsrc/usb_names.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- src/gamepad_renderer.h
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/input_backend.h (1)
65-65: Add documentation clarifyingGetSlotDisplayNamepointer lifetime.The returned
const char*pointers from current implementations (which delegate toGetFriendlyName) are safe—they reference compile-time string literals in a static array. However, the base class method lacks explicit lifetime documentation, which could lead future overrides to accidentally return dangling pointers from temporary/local storage.Suggested documentation
- [[nodiscard]] virtual const char* GetSlotDisplayName([[maybe_unused]] int slot) const { return nullptr; } + /// `@returns` A null-terminated C-string containing the slot display name, or `nullptr` if none is available. + /// The returned pointer must remain valid at least until the next `Poll()` call (or backend destruction). + [[nodiscard]] virtual const char* GetSlotDisplayName([[maybe_unused]] int slot) const { return nullptr; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/input_backend.h` at line 65, The base method GetSlotDisplayName currently returns a raw const char* but has no doc on required lifetime; add a brief comment on the declaration of GetSlotDisplayName clarifying that the returned pointer must remain valid for the caller (e.g., point to a static string literal or other storage that outlives the call/object) and must not point into temporary or local buffers; reference existing delegating implementations (which use GetFriendlyName and static string arrays) as examples and suggest alternatives for overrides (persistent storage or std::string_view/owner-managed memory) to prevent dangling pointers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/input_backend.h`:
- Line 65: The base method GetSlotDisplayName currently returns a raw const
char* but has no doc on required lifetime; add a brief comment on the
declaration of GetSlotDisplayName clarifying that the returned pointer must
remain valid for the caller (e.g., point to a static string literal or other
storage that outlives the call/object) and must not point into temporary or
local buffers; reference existing delegating implementations (which use
GetFriendlyName and static string arrays) as examples and suggest alternatives
for overrides (persistent storage or std::string_view/owner-managed memory) to
prevent dangling pointers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 25079ce4-59ac-45ca-a8bf-e4974d271207
📒 Files selected for processing (2)
src/gamepad_renderer.cppsrc/input_backend.h
This was referenced Mar 15, 2026
Merged
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary by CodeRabbit