Skip to content

refactor: use constants instead of magic numbers, and a single range …#147

Merged
gmegidish merged 2 commits intomainfrom
refactor-use-constants-and-single-port-range
Jan 18, 2026
Merged

refactor: use constants instead of magic numbers, and a single range …#147
gmegidish merged 2 commits intomainfrom
refactor-use-constants-and-single-port-range

Conversation

@gmegidish
Copy link
Member

@gmegidish gmegidish commented Jan 18, 2026

…of ports

Summary by CodeRabbit

  • Refactor
    • Port allocation now uses configurable ranges for more reliable and flexible connections.
    • Centralized timeout settings to improve startup and broadcast wait behavior.
  • Bug Fixes / Stability
    • Improved port-forwarding for device HTTP and video streams, reducing startup failures.
    • Updated logging for clearer startup and connection status messages.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 18, 2026

Caution

Review failed

The pull request is closed.

Walkthrough

Centralizes port and timeout configuration in the iOS device handler by adding portRangeStart/portRangeEnd and deviceKit-specific port/timeouts, replaces all findAvailablePort calls with findAvailablePortInRange, removes the old finder, and updates DeviceKit and agent startup flows to use the new forwarding and timeout logic.

Changes

Cohort / File(s) Summary of changes
iOS device handler
devices/ios.go
Adds constants: portRangeStart, portRangeEnd, deviceKitHTTPPort, deviceKitStreamPort, deviceKitAppLaunchTimeout, deviceKitBroadcastTimeout. Replaces findAvailablePort with findAvailablePortInRange(start,end) across StartAgent, StartDeviceKit, StartScreenCapture flows. Removes old findAvailablePort. Updates forwarder setup to bind HTTP to deviceKitHTTPPort and stream to deviceKitStreamPort, replaces hard-coded ports (12004/12005) and fixed 5s wait with new constants, and updates log messages accordingly.
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main refactoring changes: replacing magic numbers with constants and implementing a single port range approach.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@devices/ios.go`:
- Around line 893-919: The two calls to findAvailablePortInRange can return the
same port causing the stream forward to fail; update the logic in the block
using findAvailablePortInRange, ios.NewPortForwarder, .Forward and .Stop so that
you either 1) allocate and immediately bind the HTTP port (call
httpForwarder.Forward using localHTTPPort) before selecting the stream port,
then find a distinct localStreamPort, or 2) if you must select both before
binding, call findAvailablePortInRange for the stream in a loop skipping
localHTTPPort to guarantee uniqueness; ensure any failure after binding (e.g.,
stream Forward fails) cleans up by calling httpForwarder.Stop(), and also on any
subsequent error stop any forwarders that were started.

@gmegidish gmegidish merged commit 36fca40 into main Jan 18, 2026
6 of 7 checks passed
@gmegidish gmegidish deleted the refactor-use-constants-and-single-port-range branch January 18, 2026 18:06
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.

1 participant