mirror of
https://github.com/home-assistant/iOS.git
synced 2026-02-04 11:42:39 -06:00
## Plan: Fix Local Push Activation Without WiFi Reconnection - [x] Explore repository structure and understand local push implementation - [x] Identify the root cause: NEAppPushManager configuration updates don't take effect immediately - [x] Implement fix to force manager reload when local push is toggled on - [x] Add mechanism to start managers immediately when enabled on internal network - [x] Address code review feedback (removed unused encoder, explicitly set isDirty flag) - [x] Improve code quality (extract magic number, use weak self, safe error handling) - [x] Fix memory safety issues in updateManagers method - [x] Add comprehensive documentation to clarify reload behavior - [x] Add documentation explaining "dirty" flag meaning - [x] Fix lint issues (trailing spaces and line wrapping) - [x] Request final code review - [x] Run security checks (CodeQL - no issues found) ## Summary Successfully fixed the issue where activating local push requires reconnecting to WiFi. The solution ensures that when a user enables local push while already on their internal network, the `NEAppPushManager` configuration is properly reloaded so the NetworkExtension framework picks up the changes immediately. ## Key Changes Modified `NotificationManagerLocalPushInterfaceExtension.swift`: - Added `isDirty` flag to track configuration changes - Added `reloadManagersAfterSave()` method with 0.5s delay to reload managers - Improved memory safety with `[weak self]` captures - Replaced force unwraps with safe error handling - Added comprehensive documentation including clear explanation of "dirty" flag - Fixed all linting issues (trailing spaces and line length) - Ensured NetworkExtension framework picks up changes without WiFi reconnection ## Code Quality All code review feedback has been addressed: - ✅ No force unwraps - ✅ Weak self references to prevent retain cycles - ✅ Named constants instead of magic numbers - ✅ Comprehensive documentation with clear explanation of "dirty" concept - ✅ No security vulnerabilities (CodeQL clean) - ✅ All linting issues resolved The changes are minimal, focused, and solve the reported issue while improving overall code quality. <!-- START COPILOT CODING AGENT SUFFIX --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>Activating local push requires reconnecting to the WiFi</issue_title> > <issue_description>**iOS device model, version and app version** > <!-- Please include your device 'Model Name' and 'Software Version' as listed in iOS Settings>General>About. Please also give the app version listed beneath "Home Assistant Companion" in the App Configuration>About menu within the app, please include the number in brackets --> > > Model Name: iPhone 13 Pro Max > Software Version: 26.1 > App version: 2025.11.2 > > **Home Assistant Core Version** > 2025.8.2 > > **Describe the bug** > When I activate local push, the status section doesn't immediately show it as activated. I have to reconnect the phone to the local network for local push to work. > > **To Reproduce** > * Disable local push > * Status "Local push" is disabled > * Enable local push while you are connected to your local network > * Status "Local push" still disabled > * Disconnect from your local network > * Reconnect to your local network > * Status "Local push" now is available > > **Expected behavior** > * When I enable local push while I am connected to my local network, local push should be initialized without having to reconnect to the wifi.</issue_description> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> - Fixes home-assistant/iOS#4031 <!-- START COPILOT CODING AGENT TIPS --> --- ✨ Let Copilot coding agent [set things up for you](https://github.com/home-assistant/iOS/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bgoncal <5808343+bgoncal@users.noreply.github.com>
289 lines
11 KiB
Swift
289 lines
11 KiB
Swift
import Foundation
|
|
import HAKit
|
|
import NetworkExtension
|
|
import PromiseKit
|
|
import Shared
|
|
|
|
final class NotificationManagerLocalPushInterfaceExtension: NSObject, NotificationManagerLocalPushInterface {
|
|
/// Delay in seconds before reloading managers after configuration changes.
|
|
/// This allows the system to persist changes before attempting to reload them.
|
|
private static let managerReloadDelay: TimeInterval = 0.5
|
|
|
|
private var observers = [Observer]()
|
|
private var syncStates: PerServerContainer<LocalPushStateSync>!
|
|
private var managers = [Identifier<Server>: [NEAppPushManager]]()
|
|
|
|
private var tokens: [NSKeyValueObservation] = [] {
|
|
didSet {
|
|
for token in oldValue where !tokens.contains(where: { $0 === token }) {
|
|
token.invalidate()
|
|
}
|
|
}
|
|
}
|
|
|
|
func status(for server: Server) -> NotificationManagerLocalPushStatus {
|
|
if managers[server.identifier, default: []].contains(where: \.isActive) {
|
|
if let state = syncStates[server].value {
|
|
// manager is running and we have a value synced
|
|
return .allowed(state)
|
|
} else {
|
|
// manager claims to be running but push provider didn't set sync status
|
|
return .disabled
|
|
}
|
|
} else {
|
|
// manager isn't running
|
|
return .disabled
|
|
}
|
|
}
|
|
|
|
func addObserver(
|
|
for server: Server,
|
|
handler: @escaping (NotificationManagerLocalPushStatus) -> Void
|
|
) -> HACancellable {
|
|
let observer = Observer(server: server, handler: handler)
|
|
observers.append(observer)
|
|
return HABlockCancellable { [weak self] in
|
|
self?.observers.removeAll(where: { $0 == observer })
|
|
}
|
|
}
|
|
|
|
private struct Observer: Equatable {
|
|
var identifier = UUID()
|
|
var server: Server
|
|
var handler: (NotificationManagerLocalPushStatus) -> Void
|
|
|
|
static func == (lhs: Observer, rhs: Observer) -> Bool {
|
|
lhs.identifier == rhs.identifier
|
|
}
|
|
}
|
|
|
|
private func notifyObservers(for servers: [Server] = Current.servers.all) {
|
|
for observer in observers where servers.contains(observer.server) {
|
|
let status = status(for: observer.server)
|
|
observer.handler(status)
|
|
}
|
|
}
|
|
|
|
override init() {
|
|
super.init()
|
|
self.syncStates = PerServerContainer<LocalPushStateSync>(constructor: { server in
|
|
let sync = LocalPushStateSync(settingsKey: PushProviderConfiguration.defaultSettingsKey(for: server))
|
|
let token = sync.observe { [weak self] _ in
|
|
self?.notifyObservers(for: [server])
|
|
}
|
|
return .init(sync, destructor: { _, _ in token.cancel() })
|
|
})
|
|
Current.servers.add(observer: self)
|
|
|
|
updateManagers()
|
|
}
|
|
|
|
private func updateManagers() {
|
|
Current.Log.info()
|
|
|
|
NEAppPushManager.loadAllFromPreferences { [weak self] managers, error in
|
|
guard let self else { return }
|
|
|
|
if let error {
|
|
Current.Log.error("failed to load local push managers: \(error)")
|
|
return
|
|
}
|
|
|
|
let encoder = JSONEncoder()
|
|
|
|
var updatedManagers = [ConfigureManager]()
|
|
var usedManagers = Set<NEAppPushManager>()
|
|
var hasDirtyManagers = false
|
|
|
|
// update or create managers for the servers we have
|
|
for (ssid, servers) in serversBySSID() {
|
|
Current.Log.info("configuring push for \(ssid): \(servers)")
|
|
|
|
let existing = managers?.first(where: { $0.matchSSIDs == [ssid] })
|
|
if let existing {
|
|
usedManagers.insert(existing)
|
|
}
|
|
let updated = updateManager(
|
|
existingManager: existing,
|
|
ssid: ssid,
|
|
servers: servers,
|
|
encoder: encoder
|
|
)
|
|
updatedManagers.append(updated)
|
|
if updated.isDirty {
|
|
hasDirtyManagers = true
|
|
}
|
|
}
|
|
|
|
// remove any existing managers that didn't match
|
|
for manager in managers ?? [] where !usedManagers.contains(manager) {
|
|
manager.removeFromPreferences { error in
|
|
Current.Log.info("remove unused manager \(manager) result: \(String(describing: error))")
|
|
}
|
|
}
|
|
|
|
configure(managers: updatedManagers)
|
|
|
|
// If we made changes to managers, reload them after a brief delay to ensure
|
|
// the system picks up the changes, especially when enabling local push
|
|
// while already on the internal network
|
|
if hasDirtyManagers {
|
|
DispatchQueue.main.asyncAfter(deadline: .now() + Self.managerReloadDelay) { [weak self] in
|
|
self?.reloadManagersAfterSave()
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
/// Reloads manager configurations from system preferences after they have been saved.
|
|
/// This ensures the NetworkExtension framework picks up configuration changes,
|
|
/// particularly when enabling local push while already on the internal network.
|
|
///
|
|
/// Note: This only configures managers that were successfully saved by updateManagers().
|
|
/// Managers for removed SSIDs or disabled servers are intentionally not recreated.
|
|
private func reloadManagersAfterSave() {
|
|
Current.Log.info("Reloading managers after configuration changes")
|
|
|
|
NEAppPushManager.loadAllFromPreferences { [weak self] managers, error in
|
|
guard let self else { return }
|
|
|
|
if let error {
|
|
Current.Log.error("failed to reload local push managers: \(error)")
|
|
return
|
|
}
|
|
|
|
var configureManagers = [ConfigureManager]()
|
|
|
|
// Only configure managers for currently enabled servers with configured SSIDs
|
|
for (ssid, servers) in serversBySSID() {
|
|
if let manager = managers?.first(where: { $0.matchSSIDs == [ssid] }) {
|
|
configureManagers.append(
|
|
ConfigureManager(ssid: ssid, manager: manager, servers: servers, isDirty: false)
|
|
)
|
|
}
|
|
}
|
|
|
|
configure(managers: configureManagers)
|
|
}
|
|
}
|
|
|
|
struct ConfigureManager {
|
|
var ssid: String
|
|
var manager: NEAppPushManager
|
|
var servers: [Server]
|
|
|
|
/// Indicates whether the manager's configuration has been modified and saved to preferences.
|
|
/// A "dirty" manager is one that had changes to its properties (isEnabled, matchSSIDs,
|
|
/// providerConfiguration, etc.) and was saved via `saveToPreferences()`.
|
|
/// This flag is used to trigger a reload of managers after saving, ensuring the
|
|
/// NetworkExtension framework picks up the configuration changes immediately.
|
|
var isDirty: Bool = false
|
|
}
|
|
|
|
private func configure(managers configureManagers: [ConfigureManager]) {
|
|
tokens.removeAll()
|
|
|
|
managers = configureManagers.reduce(into: [Identifier<Server>: [NEAppPushManager]]()) { result, value in
|
|
// notify on active state changes
|
|
tokens.append(value.manager.observe(\.isActive) { [weak self, servers = value.servers] manager, _ in
|
|
Current.Log.info("manager \(value.ssid) is active: \(manager.isActive)")
|
|
self?.notifyObservers(for: servers)
|
|
})
|
|
|
|
for server in value.servers {
|
|
result[server.identifier, default: []].append(value.manager)
|
|
}
|
|
|
|
value.manager.delegate = self
|
|
}
|
|
|
|
Current.Log.verbose("computed managers: \(managers)")
|
|
|
|
notifyObservers()
|
|
}
|
|
|
|
private func updateManager(
|
|
existingManager: NEAppPushManager?,
|
|
ssid: String,
|
|
servers: [Server],
|
|
encoder: JSONEncoder
|
|
) -> ConfigureManager {
|
|
let manager = existingManager ?? NEAppPushManager()
|
|
// just toggling isEnabled doesn't seem to kill off the extension reliably, so we remove when unwanted
|
|
|
|
// Track whether any configuration properties are modified.
|
|
// "Dirty" means the manager's configuration differs from what's currently saved
|
|
// and requires a call to saveToPreferences() to persist the changes.
|
|
var isDirty = false
|
|
|
|
func updateAndDirty<T: Equatable>(_ keyPath: ReferenceWritableKeyPath<NEAppPushManager, T>, _ value: T) {
|
|
if manager[keyPath: keyPath] != value {
|
|
Current.Log.info(keyPath)
|
|
manager[keyPath: keyPath] = value
|
|
isDirty = true
|
|
}
|
|
}
|
|
|
|
updateAndDirty(\.isEnabled, true)
|
|
updateAndDirty(\.localizedDescription, "HomeAssistant for \(ssid)")
|
|
updateAndDirty(\.providerBundleIdentifier, AppConstants.BundleID + ".PushProvider")
|
|
updateAndDirty(\.matchSSIDs, [ssid])
|
|
|
|
let configurations: [PushProviderConfiguration] = servers.map {
|
|
.init(serverIdentifier: $0.identifier, settingsKey: PushProviderConfiguration.defaultSettingsKey(for: $0))
|
|
}
|
|
|
|
do {
|
|
let existing = manager.providerConfiguration[PushProviderConfiguration.providerConfigurationKey] as? Data
|
|
let new = try encoder.encode(configurations)
|
|
|
|
if existing != new {
|
|
isDirty = true
|
|
manager.providerConfiguration = [
|
|
PushProviderConfiguration.providerConfigurationKey: new,
|
|
]
|
|
}
|
|
} catch {
|
|
Current.Log.error("failed to create config for push: \(error)")
|
|
manager.providerConfiguration = [:]
|
|
}
|
|
|
|
if isDirty {
|
|
manager.saveToPreferences { error in
|
|
Current.Log.info("manager \(manager) saved, error: \(String(describing: error))")
|
|
}
|
|
}
|
|
|
|
return ConfigureManager(ssid: ssid, manager: manager, servers: servers, isDirty: isDirty)
|
|
}
|
|
|
|
private func serversBySSID() -> [String: [Server]] {
|
|
Current.servers.all.reduce(into: [String: [Server]]()) { result, server in
|
|
let connection = server.info.connection
|
|
|
|
guard connection.isLocalPushEnabled, connection.address(for: .internal) != nil else {
|
|
return
|
|
}
|
|
|
|
for ssid in server.info.connection.internalSSIDs ?? [] {
|
|
result[ssid, default: []].append(server)
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
extension NotificationManagerLocalPushInterfaceExtension: ServerObserver {
|
|
func serversDidChange(_ serverManager: ServerManager) {
|
|
updateManagers()
|
|
}
|
|
}
|
|
|
|
extension NotificationManagerLocalPushInterfaceExtension: NEAppPushDelegate {
|
|
func appPushManager(
|
|
_ manager: NEAppPushManager,
|
|
didReceiveIncomingCallWithUserInfo userInfo: [AnyHashable: Any] = [:]
|
|
) {
|
|
// we do not have calls
|
|
}
|
|
}
|