Skip to content

Support https/wss and other Add Device UI improvements#87

Open
smitty078 wants to merge 8 commits into
Moustachauve:mainfrom
smitty078:add-tls-connection
Open

Support https/wss and other Add Device UI improvements#87
smitty078 wants to merge 8 commits into
Moustachauve:mainfrom
smitty078:add-tls-connection

Conversation

@smitty078
Copy link
Copy Markdown

@smitty078 smitty078 commented May 13, 2026

Implements the following changes

URL Handling

  • Normalize and store canonical device base URLs instead of raw hostnames/IPs
  • Add centralized URL and websocket URL handling (http/https → ws/wss)
  • Update webview, websocket, and OTA update flows to consume canonical URLs directly

User Interface

  • DeviceAddView:

    • Supports manual entry of full URLs, ports, and HTTPS devices
    • Supports entering Custom Name right from device add
    • Displayed as NavigationStack/Group instead of Navigation/VStack, which makes it behave more modern and native
  • DeviceInfoTwoRows:

    • Truncates long URLs so that the view width never gets clipped in narrow parent views

@smitty078 smitty078 marked this pull request as draft May 13, 2026 15:14
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces the ControllerAddress utility to handle URL normalization and WebSocket endpoint generation. The review feedback suggests adding validation to ensure only 'http' or 'https' schemes are accepted to prevent errors in WebSocket URL construction, and recommends replacing the deprecated appendingPathComponent method with the modern appending(path:) API.

Comment thread ControllerAddress.swift Outdated
Comment thread ControllerAddress.swift Outdated
@smitty078 smitty078 force-pushed the add-tls-connection branch 10 times, most recently from 962d280 to 8cb8461 Compare May 14, 2026 20:31
@smitty078 smitty078 force-pushed the add-tls-connection branch from 8cb8461 to 9d29682 Compare May 14, 2026 20:36
@smitty078
Copy link
Copy Markdown
Author

/gemini review

@smitty078
Copy link
Copy Markdown
Author

/gemini summary

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request implements support for secure connections (HTTPS/WSS) and improves device address normalization across the application. Key changes include the addition of computed URL and WebSocket URL properties to the Device model, a redesigned device addition interface featuring a secure connection toggle and custom naming, and fixes for path construction in the update service. Review feedback identifies an empty localization entry for 'Device Added' and recommends refactoring duplicated address normalization logic into a shared utility to improve maintainability.

Comment thread wled/Localizable.xcstrings
Comment thread wled/View/DeviceAdd/DeviceAddViewModel.swift
@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

This pull request introduces robust support for secure connections (HTTPS/WSS) by standardizing how device URLs are stored and processed throughout the application. It refactors the device addition flow to provide a more native user experience, including manual URL entry, custom naming, and better feedback during the connection process. These changes ensure consistent URL handling across webviews, websocket clients, and OTA update services.

Highlights

  • URL Normalization: Implemented centralized canonical URL handling, replacing raw hostnames with full URL objects to support both http and https/wss protocols.
  • Device Addition UI: Modernized the Device Add flow using NavigationStack, added support for custom device names, and included a secure connection toggle.
  • UI/UX Improvements: Enhanced device information display with text truncation to prevent layout clipping and added localized warning banners for local network permissions.
Activity
  • Initial pull request created by smitty078.
  • Automated review identified issues with URL scheme validation and deprecated API usage.
  • Automated review highlighted duplicate normalization logic and missing localization strings.
  • Author requested a summary of the changes.

@smitty078
Copy link
Copy Markdown
Author

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request enhances device management by introducing a centralized DeviceAddressNormalizer and updating the Device model to provide computed url and webSocketURL properties. The "Add Device" interface has been refactored to support custom device names and a toggle for secure connections, while the underlying services now utilize the normalized address logic. Additionally, the PR fixes a file path bug in the update service and improves the UI for device status and addition. Review feedback highlights a potential desync in the secure connection toggle logic and identifies localization issues with interpolated strings in SwiftUI views.

Comment thread wled/View/DeviceAdd/DeviceAddView.swift
Comment thread wled/View/DeviceAdd/DeviceAddView.swift Outdated
Comment thread wled/View/DeviceAdd/DeviceAddView.swift Outdated
smitty078 and others added 3 commits May 14, 2026 21:55
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@smitty078 smitty078 marked this pull request as ready for review May 15, 2026 01:58
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