Conversation
There was a problem hiding this comment.
Pull Request Overview
Enhances notification handling by introducing a structured payload model, improving UI display with icons, and refactoring various notification-related components.
- Introduces
PushNotificationPayloadfor structured notification data parsing - Adds notification icon display support and image fetching functionality
- Updates notification models and API methods for better data handling
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| openHAB/AppDelegate.swift | Adds image fetching for notification icons and updates notification display logic |
| openHAB/NotificationsView.swift | Updates UI state management and API method names |
| openHAB/OpenHABRootViewController.swift | Fixes typo in comment |
| OpenHABCore/Sources/OpenHABCore/Model/PushNotificationPayload.swift | New structured payload model for push notifications |
| OpenHABCore/Sources/OpenHABCore/Model/OpenHABNotification.swift | Restructures notification model with expanded payload support |
| OpenHABCore/Sources/OpenHABCore/Util/HTTPClient.swift | Renames method for consistency |
| OpenHABCore/Sources/OpenHABCore/Util/Endpoint.swift | Adds default parameters to icon method |
| openHAB.xcodeproj/project.pbxproj | Changes code signing configuration |
| fastlane/Fastfile | Updates changelog format |
| CHANGELOG.md | Adds recent change entries |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
I overlooked NotificationService, maybe because it never worked so far for me. |
Not sure i understand, push notifications in general don't work for you ? |
|
Push notification work for me but the attached image is never displayed to me |
Is the image a openHAB image item or are you referencing a URL? It works for me consistently, would like to track down why its not working for you. FYI the "icon" option in the cloud actions does not work on iOS, maybe it did at one point long ago before i inherited it, but there is not a way to show an icon in an actual native iOS notification short of attaching it as a image which doesn't really make sense in practice. |
|
Can you please share an example call from a openhab rule where you send a notification |
|
Here's a notification when someone rings my front gate. actions.notificationBuilder("Someone is at the Front Gate")
.addUserId(email)
.withTitle('Front Gate')
.withTag('front-gate-activity')
.withReferenceId('front-gate-activity')
.withOnClickAction('ui:popup:widget:gate_control')
.withMediaAttachmentUrl('https://www.xxx.com/xxx/front-gate')
.addActionButton('Open Gate','command:GDSDrivewayGateSwitch:ON')
.addActionButton('Answer Call','ui:popup:widget:gate_control_auto')
.send();where I was actually just looking at changing the core HTTP rule actions to include an option to download and set an image to an item to make this easier for people, that would probably make this feature more accessible. |
|
@timbms if you are running 5.0.x nightlies, i added a new HTTP rule action to help with using images in notifications, you can grab a local image and set it as the state of an Item, then use that in the notification. This will work through myopenHAB as well a local, also has the advantage of capturing the image as the even happens, where a notification could be several seconds later and if hitting the camera directly, could miss the event in the image (which is why i will start using this) These are taken from the cloud binding README Rules DSL rule "Motion Detected Notification"
when
Item Apartment_MotionSensor changed to ON
then
setImage("Apartment_Camera_Snapshot", "http://camera.local/camera-snapshot.jpg");
sendBroadcastNotification("Motion detected in the apartment!", "motion", "Motion Tag",
"Motion Detected", "motion-id-1234", null, "item:Apartment_Camera_Snapshot",
"Turn on the light=command:Apartment_Light:ON", null, null)
endJavascript Rules rules.when().item('Apartment_MotionSensor').changed().to('ON').then(() => {
actions.HTTP.setImage("Apartment_Camera_Snapshot", "http://camera.local/camera-snapshot.jpg");
actions.notificationBuilder('Motion detected in the apartment!')
.withIcon('motion')
.withTag('Motion Tag')
.withTitle('Motion Detected')
.withReferenceId('motion-id-1234')
.withMediaAttachment('item:Apartment_Camera_Snapshot')
.addActionButton('Turn on the light', 'command:Apartment_Light:ON')
.send();
}).build('Motion Detected Notification');Ruby Rules rule "Motion Detected Notification" do
changed Apartment_MotionSensor, to: ON
run do
HTTP.setImage("Apartment_Camera_Snapshot", "http://camera.local/camera-snapshot.jpg");
Notification.send "Motion detected in the apartment!",
icon: "motion",
tag: "Motion Tag",
title: "Motion Detected",
id: "motion-id-1234"
attachment: "item:Apartment_Camera_Snapshot",
buttons: { "Turn on the light" => "command:Apartment_Light:ON" }
end
end |
|
Cool feature @digitaldan |
|
Hey @timbms i'm back from a trip and back working on openHAB, do you need anything from me on this PR? |
|
Hi @digitaldan good you are back. I was also off in remote Swiss mountains. |
The fix on the other notification PR that just got merged will address this. Hope you had a good holiday! My question was on this PR, are you waiting for a review, or is there more work to do? |
| options.append(contentsOf: extraOptions) | ||
|
|
||
| // Async Kingfisher call → gives you RetrieveImageResult with a UIImage. | ||
| let result = try await KingfisherManager.shared.retrieveImage(with: url, options: options) |
There was a problem hiding this comment.
I don't think this will work with setups with authentication, client certs or myopenhab ?
Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Alignment with develop Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: Tim Bert <5411131+timbms@users.noreply.github.com>
Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
… to obtain wider segmented buttons Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Update logic skips refreshing the widget's type, leading to stale types showing as switches instead of charts; adding type assignment during updates should fix the display inconsistency. Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
* Fix screensaver layout sizing --------- Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Replace NumberFormatter (with invalid "US" locale) with Float(_:format:) using en_US_POSIX for reliable decimal parsing. Add CGFloatExtensionTests covering basic division, decimal input, locale robustness, invalid strings, and edge cases. Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
…ingTests Replace legacy DateFormatter.iso8601Full with JSONDecoder.makeISO8601TolerantDecoder() which uses Date.ISO8601FormatStyle. Update HTTPClient to use the new decoder. Add comprehensive DateFormattingTests covering ISO8601 parsing, round-trip, JSON decoding, log formatting, screensaver time, and notification timestamp formats. Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Replace legacy regex and NumberFormatter usage in StringExtension with modern Swift equivalents. Add comprehensive StringExtensionTests covering doubleValue, intValue, numberValue, and asDouble parsing behaviour. Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Without it, internal members (doubleValue, intValue, etc.) are inaccessible from the test target. Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
The ASAN configuration was the only one, forcing all runs to use Address Sanitizer. This caused 'Undefined symbol: ___asan_alloca_poison' because the host app binary isn't built with ASAN instrumentation. Add a plain default 'Test' configuration; ASAN remains available for explicit use. Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Add OTHER_LDFLAGS = -undefined dynamic_lookup to the test target so the linker defers resolution of dynamic framework symbols (SDWebImageSVGCoder, etc.) to runtime, where they are already loaded by the host app. Also remove the redundant SDImageSVGCoder registration in OpenHABSVGTests setUpWithError — AppDelegate already registers the coder before tests run. Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Replace 'import SDWebImageSVGCoder' with ObjC runtime lookup via
NSClassFromString("SDImageSVGCoder"). This eliminates the hard link-time
dependency on SDWebImage data symbols (_SDImageCoderDecodePreserveAspectRatio
etc.) that BUNDLE_LOADER and -undefined dynamic_lookup cannot reliably resolve.
The coder is already registered by AppDelegate before any tests run,
so runtime lookup is sufficient. Also revert the -undefined dynamic_lookup
OTHER_LDFLAGS added in the previous commit.
Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
- Add OpenAPIErrorInspector utility to OpenHABCore, centralising all ClientError unwrapping so callers don't need to import OpenAPIRuntime - Remove OpenAPIRuntime imports from SitemapPageViewModel and SingleConnectionSettingsView; replace ClientError casts with inspector - Add SDWebImage as explicit Package.swift dependency (required now that OpenHABCore uses SDWebImage APIs directly) - Add process(data:) convenience on OpenHABImageProcessor for test use - Migrate OpenHABSVGTests from XCTest to Swift Testing (@Suite/@Test/#expect) and replace fragile ObjC-runtime SDImageSVGCoder lookup with direct OpenHABImageProcessor call via @testable import OpenHABCore - Add dedicated openHABTestsSwift.xctestplan; point openHABTestsSwift scheme at it; remove ASAN config from openHABTests plan Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
The Main UI script editor height does not account for the bottom toolbar's safe area padding on iPhone, causing the last 2-3 lines to be hidden behind the toolbar and unreachable by scrolling. Inject a JavaScript fix via WKWebView that adds bottom padding to the page content and CodeMirror scroll container equal to the native safe area inset plus any FAB button height. A MutationObserver re-applies the fix on SPA navigation back to the script editor page. Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
CI uses testplan: 'openHABTests' in the Fastfile unittests lane, but the scheme only had openHABTestsSwift.xctestplan associated, causing xcodebuild to error with 'Scheme does not have an associated test plan named openHABTests'. Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
ASan instrumentation causes undefined symbol errors (___asan_alloca_poison) when Swift Package Manager dependencies are not recompiled with ASan support. Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: openhab-bot <info@openhabfoundation.org>
…1102) * Fix empty string commands and switch sendItemCommand to JSON payload Text input items can now be cleared by submitting an empty string. Previously, empty commands were silently dropped in both InputCommandFormatter and WidgetCommandDispatcher. The guard order in InputCommandFormatter.command() is corrected so that only number hints reject empty input; text hints pass through an empty string as a valid "clear" command. Also fixes the openapi.json schema for sendItemCommand, which incorrectly declared the application/json body as type:string. It is now type:object with a required "value" field, matching the actual server contract. Types.swift and OpenAPIService are regenerated/updated accordingly. --------- Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: openhab-bot <info@openhabfoundation.org>
Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com> Improve parsing for notifications Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Comprehensive parsing of notifications Improved error handling Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
3a94e5f to
2829639
Compare
1f242b5 to
166a59c
Compare
Uh oh!
There was an error while loading. Please reload this page.