Skip to content

Comments

Add Swift modernization foundation with testing infrastructure#3

Open
rosbel wants to merge 6 commits intomasterfrom
claude/modernize-swift-HzTKA
Open

Add Swift modernization foundation with testing infrastructure#3
rosbel wants to merge 6 commits intomasterfrom
claude/modernize-swift-HzTKA

Conversation

@rosbel
Copy link
Owner

@rosbel rosbel commented Feb 3, 2026

  • Add bridging headers for Swift/Obj-C interoperability
  • Create OBDTypes.swift with protocol types (PIDs, BLEDataPacket, DiagnosticReading, VehicleDiagnostics)
  • Create BLETypes.swift with BLE state management types
  • Create DateFormatting.swift utilities replacing UtilityMethods
  • Add comprehensive test suite with 70+ test cases
  • Add TestHelpers.swift with test data builders

https://claude.ai/code/session_01FRVeNk1DQRq6pjo1czA1YE

- Add bridging headers for Swift/Obj-C interoperability
- Create OBDTypes.swift with protocol types (PIDs, BLEDataPacket, DiagnosticReading, VehicleDiagnostics)
- Create BLETypes.swift with BLE state management types
- Create DateFormatting.swift utilities replacing UtilityMethods
- Add comprehensive test suite with 70+ test cases
- Add TestHelpers.swift with test data builders

https://claude.ai/code/session_01FRVeNk1DQRq6pjo1czA1YE
BLEManager (vBox/Managers/BLEManagerSwift.swift):
- Modern Swift implementation with Combine publishers
- CBCentralManager and CBPeripheralManager delegates
- Reactive state management ($state, $isConnected, $diagnostics)
- Speed, RPM, fuel level publishers for UI binding
- Delegate protocol for Obj-C compatibility
- Thread-safe BLE operations on dedicated queues

Core Data Models (vBox/Models/CoreData/):
- TripEntity: Trip metadata, duration calculation, statistics, path coordinates
- GPSLocationEntity: Location storage, CLLocation conversion, distance/bearing calculation
- BluetoothDataEntity: OBD-II diagnostics, temperature monitoring, accelerometer
- DrivingHistoryEntity: Trip management, grouped queries, statistics aggregation

Tests (100+ new test cases):
- BLEManagerSwiftTests: Singleton, state, Combine publishers, delegate
- CoreDataModelTests: In-memory context, all entity operations, computed properties

https://claude.ai/code/session_01FRVeNk1DQRq6pjo1czA1YE
Parse shut down in 2017; all this code was non-functional.

Removed:
- Parse.framework, Bolts.framework, ParseCrashReporting.framework
- Parse imports from AppDelegate.m and GoogleMapsViewController.m
- Parse initialization, crash reporting, and API keys from AppDelegate
- PFAnalytics tracking calls from GoogleMapsViewController
- PFInstallation badge management from applicationDidBecomeActive
- PFPush notification handling
- Parse symbols upload shell script from build phases
- All Parse framework references from project.pbxproj

Updated:
- CLAUDE.md to reflect Swift modernization and remove Parse references
- AppDelegate now only initializes Google Maps SDK
- GoogleMapsViewController no longer tracks analytics

https://claude.ai/code/session_01FRVeNk1DQRq6pjo1czA1YE
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Introduces a Swift modernization foundation for the vBox app by adding Swift equivalents for core BLE/OBD/domain types, date formatting utilities, and a large new XCTest suite, alongside bridging headers for Obj-C/Swift interoperability.

Changes:

  • Added Swift domain models for OBD-II packets/readings and BLE state/type handling, plus a new BLEManagerSwift implementation using Combine publishers.
  • Added Swift date/time formatting utilities intended to replace UtilityMethods.
  • Added extensive XCTest coverage and shared test helpers, including an in-memory Core Data test harness.

Reviewed changes

Copilot reviewed 65 out of 72 changed files in this pull request and generated 22 comments.

Show a summary per file
File Description
vBox/vBox-Bridging-Header.h Adds app-target bridging header importing Obj-C app types into Swift.
vBoxTests/vBoxTests-Bridging-Header.h Adds test-target bridging header importing Obj-C app types into Swift tests.
vBox/Models/OBDTypes.swift Introduces Swift OBD PID definitions, BLE packet parsing, and diagnostics snapshot types.
vBox/Models/BLETypes.swift Introduces Swift BLE state/peripheral/error types and a Swift delegate protocol.
vBox/Managers/BLEManagerSwift.swift Adds a new Swift BLE manager with Combine publishers and delegate callbacks.
vBox/Utilities/DateFormatting.swift Adds Swift duration/date display formatting utilities and related extensions.
vBox/Models/CoreData/TripEntity.swift Adds Swift NSManagedObject subclass for Trip with computed properties and factories.
vBox/Models/CoreData/GPSLocationEntity.swift Adds Swift NSManagedObject subclass for GPSLocation with factories and helpers.
vBox/Models/CoreData/DrivingHistoryEntity.swift Adds Swift NSManagedObject subclass for DrivingHistory with stats and factories.
vBox/Models/CoreData/BluetoothDataEntity.swift Adds Swift NSManagedObject subclass for BluetoothData with computed properties and factories.
vBoxTests/TestHelpers.swift Adds shared builders/utilities for tests (BLE packets, locations, diagnostics, assertions).
vBoxTests/OBDTypesTests.swift Adds unit tests for OBD/BLE packet parsing, reading validation, and diagnostics updates.
vBoxTests/BLETypesTests.swift Adds unit tests for Swift BLE types (state, peripheral type, signal strength, errors).
vBoxTests/BLEManagerSwiftTests.swift Adds tests for BLEManagerSwift publishers/delegate wiring and data packet builders.
vBoxTests/DateFormattingTests.swift Adds tests for duration/date formatting utilities and Obj-C compatibility expectations.
vBoxTests/CoreDataModelTests.swift Adds in-memory Core Data tests for the new Swift Core Data entity classes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +254 to +265
func testFullDateTimeMatchesObjCFormat() {
// The Obj-C format is "MMMM dd, yyy (EEEE) HH:mm:ss"
// Note: There's a typo in the Obj-C ("yyy" instead of "yyyy") but it still works
let date = createDate(year: 2024, month: 3, day: 15, hour: 14, minute: 30, second: 45)
let result = DateDisplayFormatter.fullDateTime(date)

// Verify it contains all expected components
XCTAssertTrue(result.contains("March"))
XCTAssertTrue(result.contains("15"))
XCTAssertTrue(result.contains("2024"))
XCTAssertTrue(result.contains("Friday"))
XCTAssertTrue(result.contains("14:30:45"))
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

These compatibility assertions also hard-code English strings ("March", "Friday") while UtilityMethods.formattedStringFromDate: and DateDisplayFormatter.fullDateTime(_:) both use DateFormatter with the current locale. This can make the test suite locale-dependent and flaky on CI. Prefer comparing full strings produced by equivalent formatters under the same locale/timezone settings.

Copilot uses AI. Check for mistakes.
Comment on lines +79 to +85
tripEntity.properties = [tripStartTime, tripEndTime, tripAvgSpeed, tripMaxSpeed, tripMinSpeed, tripTotalMiles, tripName]

// GPSLocation entity
let gpsLocationEntity = NSEntityDescription()
gpsLocationEntity.name = "GPSLocation"
gpsLocationEntity.managedObjectClassName = NSStringFromClass(GPSLocationEntity.self)

Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The test model only adds attributes for Trip, GPSLocation, and BluetoothData but omits the relationships that the Swift model classes declare (TripEntity.drivingHistory / gpsLocations, GPSLocationEntity.tripInfo / bluetoothInfo, BluetoothDataEntity.location). Any test that touches locationCount, hasBluetoothData, or relationship accessors will crash unless these relationships (and inverse relationships) are added to the in-memory model.

Copilot uses AI. Check for mistakes.
Comment on lines +193 to +211
func wait(
for condition: @escaping () -> Bool,
timeout: TimeInterval = 5.0,
pollInterval: TimeInterval = 0.1,
description: String = "Condition"
) {
let expectation = XCTestExpectation(description: description)

let startTime = Date()
while !condition() {
if Date().timeIntervalSince(startTime) > timeout {
XCTFail("\(description) timed out after \(timeout) seconds")
return
}
RunLoop.current.run(until: Date(timeIntervalSinceNow: pollInterval))
}

expectation.fulfill()
}
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

XCTestCase.wait(for:timeout:pollInterval:description:) creates an XCTestExpectation but never waits on it, so the expectation is effectively unused. This makes the helper misleading and may hide issues where the run loop isn’t being driven as expected. Either remove the expectation entirely or implement this using XCTWaiter/wait(for:timeout:) so the helper behaves like a proper XCTest wait.

Copilot uses AI. Check for mistakes.
Comment on lines +16 to +24
// MARK: - Raw Values (matching Obj-C BLEState enum)

func testRawValues() {
XCTAssertEqual(BLEState.unknown.rawValue, 0)
XCTAssertEqual(BLEState.resetting.rawValue, 1)
XCTAssertEqual(BLEState.unsupported.rawValue, 2)
XCTAssertEqual(BLEState.unauthorized.rawValue, 3)
XCTAssertEqual(BLEState.off.rawValue, 4)
XCTAssertEqual(BLEState.on.rawValue, 5)
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The comment and assertions claim these raw values match the Obj-C BLEState enum, but vBox/BLEManager.h defines BLEStateOn = 0, BLEStateOff = 1, ... (different ordering). These expectations instead match CBManagerState numeric values. Either update the test description (and any interop assumptions) or introduce an explicit mapping layer if Swift needs to interoperate with the Obj-C BLEState.

Copilot uses AI. Check for mistakes.
Comment on lines +298 to +316
log("Discovered: \(localName ?? peripheral.identifier.uuidString) (RSSI: \(RSSI))")

// Stop scanning and connect
centralManager.stopScan()
isScanning = false

DispatchQueue.main.async { [weak self] in
guard let self = self else { return }
self.delegate?.bleManagerDidStopScanning?(self)
}

connectedPeripheral = peripheral
peripheral.delegate = self

centralManager.connect(
peripheral,
options: [CBConnectPeripheralOptionNotifyOnDisconnectionKey: true]
)
}
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

discoveredPeripherals is cleared in scan(for:), but didDiscover never appends to it (and immediately stops scanning + connects to the first peripheral). This makes the discoveredPeripherals publisher and connect(to: DiscoveredPeripheral) API effectively unusable. Either publish discovered devices (append and let the caller choose when to connect) or remove the unused state/API to match the intended behavior.

Copilot uses AI. Check for mistakes.
Comment on lines +253 to +256
/// Create a new BluetoothData entity
static func create(in context: NSManagedObjectContext) -> BluetoothDataEntity {
return BluetoothDataEntity(context: context)
}
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

BluetoothDataEntity(context:) will only succeed once the Core Data model associates the BluetoothData entity with the BluetoothDataEntity class. The current model still uses the Obj-C BluetoothData class, so these factories would fail at runtime unless the .xcdatamodeld is updated/migrated. Consider keeping these factories out of the main target until the model migration is in place.

Copilot uses AI. Check for mistakes.
Comment on lines +239 to +249
/// Get or create the singleton DrivingHistory instance
static func getOrCreate(in context: NSManagedObjectContext) -> DrivingHistoryEntity {
let request = fetchRequest()
request.fetchLimit = 1

if let existing = try? context.fetch(request).first {
return existing
}

let history = DrivingHistoryEntity(context: context)
return history
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

DrivingHistoryEntity(context:) will only work once the .xcdatamodeld maps the DrivingHistory entity to the Swift DrivingHistoryEntity class. With the current model still using the Obj-C DrivingHistory class, getOrCreate(in:) would fail at runtime if used in production before the model migration lands.

Copilot uses AI. Check for mistakes.
Comment on lines +14 to +20
enum BLEState: Int, CustomStringConvertible {
case unknown = 0
case resetting = 1
case unsupported = 2
case unauthorized = 3
case off = 4
case on = 5
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

BLEState here will collide with the existing Objective-C BLEState enum declared in vBox/BLEManager.h once the bridging header imports BLEManager.h (see vBox/vBox-Bridging-Header.h). This will cause a Swift compile-time redeclaration error. Consider renaming the Swift type (or the Obj-C one), or isolating these Swift BLE types into a separate module/namespace and adding explicit mapping between Obj-C and Swift states.

Copilot uses AI. Check for mistakes.
Comment on lines +235 to +243
/// Protocol for receiving BLE manager updates
protocol BLEManagerDelegate: AnyObject {
/// Called when Bluetooth state changes
func bleManager(_ manager: Any, didUpdateState state: BLEState)

/// Called when a peripheral is discovered
func bleManager(_ manager: Any, didDiscover peripheral: DiscoveredPeripheral)

/// Called when connected to a peripheral
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

This Swift BLEManagerDelegate protocol conflicts with the Objective-C @protocol BLEManagerDelegate in vBox/BLEManager.h (imported via the bridging header), which will prevent compilation. Also, using Any for the manager parameter makes the API hard to use safely; prefer a concrete manager type or an associated type once the naming collision is resolved.

Copilot uses AI. Check for mistakes.
Comment on lines +244 to +246
// Update our diagnostics snapshot
diagnostics.update(with: reading)

Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

diagnostics.update(with:) mutates the @Published diagnostics value from whatever queue didUpdateValueFor runs on (the CoreBluetooth delegate queue). Like state, this will publish from a background queue. Consider updating diagnostics on the main thread (or protecting it via @MainActor) to avoid races and ensure subscribers receive values on the expected scheduler.

Copilot uses AI. Check for mistakes.
- Update deployment target from iOS 9.0 to iOS 13.0 (required for Combine)
- Add SWIFT_VERSION = 5.0 to all build configurations
- Update Podfile with use_frameworks! and modern target structure
- Add NSBluetoothAlwaysUsageDescription for iOS 13+ BLE requirements
- Remove remote-notification background mode (Parse SDK removed)
- Update UIRequiredDeviceCapabilities from armv7 to arm64
- Update CLAUDE.md with new system requirements

https://claude.ai/code/session_01FRVeNk1DQRq6pjo1czA1YE
Convert all 6 view controllers from Objective-C to modern Swift:

- DebugBluetoothViewControllerSwift: BLE debug console with Combine
- MainScreenViewControllerSwift: Home screen with button styling
- BluetoothTableViewControllerSwift: OBD diagnostic table with Combine
- DrivingHistoryViewControllerSwift: Trip list grouped by date
- TripDetailViewControllerSwift: Trip playback with map, gauges, email sharing
- GoogleMapsViewControllerSwift: Main trip recording with GPS and BLE

Key improvements:
- Use Combine publishers for reactive BLE state management
- Modern Swift patterns (guard, optionals, closures)
- Proper memory management with weak references
- UIAlertController instead of deprecated UIAlertView
- Type-safe enums and structs

Update bridging header and CLAUDE.md documentation.

https://claude.ai/code/session_01FRVeNk1DQRq6pjo1czA1YE
- Update all 6 view controller customClass attributes to Swift versions
- Add customModule="vBox" and customModuleProvider="target" for Swift
- Fix outlet names to match storyboard connections:
  - RPMGauge (TripDetailViewController)
  - MapView (GoogleMapsViewController)

Updated controllers:
- DrivingHistoryViewControllerSwift
- MainScreenViewControllerSwift
- BluetoothTableViewControllerSwift
- DebugBluetoothViewControllerSwift
- TripDetailViewControllerSwift
- GoogleMapsViewControllerSwift

https://claude.ai/code/session_01FRVeNk1DQRq6pjo1czA1YE
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.

2 participants