-
-
Notifications
You must be signed in to change notification settings - Fork 228
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
action command prompts #1661
action command prompts #1661
Changes from 5 commits
ffee138
1ad7051
ba2a3a5
2c69c5b
242e395
b9eb29c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,8 +3,9 @@ import { Router } from '@angular/router'; | |
import { Subscription } from 'rxjs'; | ||
|
||
import { ConfigService } from './config/config.service'; | ||
import { PrinterEvent } from './model/event.model'; | ||
import { PrinterEvent, PrinterNotification } from './model/event.model'; | ||
import { SocketService } from './services/socket/socket.service'; | ||
import { NotificationService } from './notification/notification.service'; | ||
|
||
@Injectable() | ||
export class EventService implements OnDestroy { | ||
|
@@ -15,35 +16,89 @@ export class EventService implements OnDestroy { | |
public constructor( | ||
private socketService: SocketService, | ||
private configService: ConfigService, | ||
private notificationService: NotificationService, | ||
private router: Router, | ||
) { | ||
this.subscriptions.add( | ||
this.socketService.getEventSubscribable().subscribe((event: PrinterEvent) => { | ||
if (event === PrinterEvent.PRINTING || event === PrinterEvent.PAUSED) { | ||
setTimeout(() => { | ||
this.printing = true; | ||
}, 500); | ||
this.socketService.getEventSubscribable().subscribe((event: PrinterEvent | PrinterNotification) => { | ||
if (this.isPrinterNotification(event)) { | ||
this.handlePrinterNotification(event); | ||
} else { | ||
setTimeout(() => { | ||
this.printing = false; | ||
}, 1000); | ||
} | ||
|
||
if (event === PrinterEvent.CLOSED) { | ||
this.router.navigate(['/standby']); | ||
} else if (event === PrinterEvent.CONNECTED) { | ||
setTimeout(() => { | ||
if (this.configService.isTouchscreen()) { | ||
this.router.navigate(['/main-screen']); | ||
} else { | ||
this.router.navigate(['/main-screen-no-touch']); | ||
} | ||
}, 1000); | ||
this.handlePrinterEvent(event); | ||
} | ||
}), | ||
); | ||
} | ||
|
||
// https://stackoverflow.com/questions/14425568/interface-type-check-with-typescript | ||
// https://stackoverflow.com/questions/8511281/check-if-a-value-is-an-object-in-javascript/8511350#8511350 | ||
private isPrinterNotification(object: any): object is PrinterNotification { | ||
return typeof object === 'object' | ||
&& object !== null | ||
&& ('text' in object || 'message' in object || 'action' in object); | ||
} | ||
|
||
private handlePrinterNotification(event: PrinterNotification): void { | ||
const messages = { | ||
'FilamentRunout T0': $localize`:@@prompt-filament-runout-t0:Filament runout detected. Ejecting filament, please wait...`, | ||
'Nozzle Parked': $localize`:@@prompt-filament-runout:A filament runout has been detected. Please remove the ejected filament, insert filament from a new spool and press Continue.`, | ||
'Continue': $localize`:@@prompt-continue:Continue`, | ||
'Paused': $localize`:@@prompt-filament-runout-resume:The filament has been primed. Do you want to continue printing?`, | ||
'PurgeMore': $localize`:@@prompt-filament-runout-purge:Purge more filament`, | ||
'Heater Timeout': $localize`:@@prompt-heater-timeout:The hotend has been disabled due to inactivity, to avoid burning the filament. Press Reheat when ready to resume.`, | ||
'Reheat': $localize`:@@prompt-reheat:Reheat`, | ||
'Reheat Done': $localize`:@@prompt-reheat-done:The hotend is now ready.`, | ||
}; | ||
if (event.action === 'close') { | ||
this.notificationService.closeNotification(); | ||
} else if (event.choices?.length > 0) { | ||
// event is action:prompt | ||
this.notificationService.setPrompt( | ||
$localize`:@@action-required:Action required`, | ||
messages[event.text] || event.text, | ||
event.choices.map(c => messages[c] || c) | ||
); | ||
} else if (event.choices?.length == 0) { | ||
// event is action:prompt without choices | ||
this.notificationService.setWarning( | ||
$localize`:@@printer-information:Printer information`, | ||
messages[event.text] || event.text | ||
); | ||
} else { | ||
// event is action:notification | ||
// this.notificationService.setInfo( | ||
// $localize`:@@printer-information:Printer information`, | ||
// messages[event.message] || event.message | ||
// ); | ||
// TODO: annoying as a notification | ||
// should be put with an autoclear timeout in the bottom-right statusline | ||
Comment on lines
+68
to
+74
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it still should be a notification, just to keep everything consistent. If this isn't too important it might be a good idea to introduce a fourth notification type with a white strip and just set a timeout here to clear the notification after 5s (is that enough time?). What do you think of that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I need to retest what exactly was being sent, but AFAIR it was every heat enable/disable message (not always coherent with the user actions) and all changes of % being broadcasted, really cluttering information. We have been using it for a month without these and it seems on-point. |
||
} | ||
} | ||
|
||
private handlePrinterEvent(event: PrinterEvent): void { | ||
if (event === PrinterEvent.PRINTING || event === PrinterEvent.PAUSED) { | ||
setTimeout(() => { | ||
this.printing = true; | ||
}, 500); | ||
} else { | ||
setTimeout(() => { | ||
this.printing = false; | ||
}, 1000); | ||
} | ||
|
||
if (event === PrinterEvent.CLOSED) { | ||
this.router.navigate(['/standby']); | ||
} else if (event === PrinterEvent.CONNECTED) { | ||
setTimeout(() => { | ||
if (this.configService.isTouchscreen()) { | ||
this.router.navigate(['/main-screen']); | ||
} else { | ||
this.router.navigate(['/main-screen-no-touch']); | ||
} | ||
}, 1000); | ||
} | ||
} | ||
|
||
ngOnDestroy(): void { | ||
this.subscriptions.unsubscribe(); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,29 @@ | ||
<div | ||
class="notification" | ||
*ngIf="!notification.choices" | ||
[ngClass]="['notification__border-' + notification.type, show ? 'notification__show' : '']" | ||
(click)="hideNotification()" | ||
> | ||
<span class="notification__heading">{{ notification.heading }}</span> | ||
<span class="notification__text">{{ notification.text }}</span> | ||
<span class="notification__close" i18n="@@tap-close">tap this card to close it</span> | ||
</div> | ||
|
||
<div | ||
class="notification-prompt" | ||
*ngIf="notification.choices?.length > 0" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't it be enough to check whether There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is just in case an empty list is sent, we would not want to display a prompt without choices |
||
[ngClass]="[show ? 'notification-prompt__show' : '']" | ||
> | ||
<div class="notification__heading">{{ notification.heading }}</div> | ||
<div class="notification__text">{{ notification.text }}</div> | ||
<div class="notification-prompt__choices"> | ||
<div | ||
class="notification-prompt__choice" | ||
*ngFor="let choice of notification.choices; index as i; first as isFirst" | ||
[ngClass]="[isFirst ? 'notification-prompt__choice-first' : '']" | ||
(click)="answerPrompt(i)" | ||
> | ||
{{ choice }} | ||
</div> | ||
</div> | ||
</div> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,10 @@ | ||
import { Component, NgZone, OnDestroy } from '@angular/core'; | ||
import { HttpClient } from '@angular/common/http'; | ||
import { Subscription } from 'rxjs'; | ||
import { catchError } from 'rxjs/operators'; | ||
|
||
import { Notification } from '../model'; | ||
import { ConfigService } from '../config/config.service'; | ||
import { NotificationService } from './notification.service'; | ||
|
||
@Component({ | ||
|
@@ -16,11 +19,17 @@ export class NotificationComponent implements OnDestroy { | |
heading: '', | ||
text: '', | ||
type: '', | ||
choices: null, | ||
closed: null, | ||
}; | ||
public show = false; | ||
|
||
public constructor(private notificationService: NotificationService, private zone: NgZone) { | ||
public constructor( | ||
private notificationService: NotificationService, | ||
private zone: NgZone, | ||
private http: HttpClient, | ||
private configService: ConfigService, | ||
) { | ||
this.subscriptions.add( | ||
this.notificationService | ||
.getObservable() | ||
|
@@ -35,6 +44,21 @@ export class NotificationComponent implements OnDestroy { | |
} | ||
} | ||
|
||
public answerPrompt(index: number): void { | ||
this.http | ||
.post( | ||
this.configService.getApiURL('plugin/action_command_prompt'), | ||
{ command: 'select', choice: index }, | ||
this.configService.getHTTPHeaders() | ||
) | ||
.pipe( | ||
catchError(error => | ||
this.notificationService.setError($localize`:@@error-answer-prompt:Can't answer prompt!`, error.message), | ||
), | ||
) | ||
.subscribe(); | ||
} | ||
Comment on lines
+48
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should be in From now on all communication with OctoPrint should be behind an abstract service with a generic interface to enable easy integration of other printer software. |
||
|
||
private setNotification(notification: Notification | 'close'): void { | ||
this.zone.run(() => { | ||
if (notification === 'close') { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,7 @@ import { webSocket, WebSocketSubject } from 'rxjs/webSocket'; | |
|
||
import { ConfigService } from '../../config/config.service'; | ||
import { ConversionService } from '../../conversion.service'; | ||
import { JobStatus, PrinterEvent, PrinterState, PrinterStatus, SocketAuth } from '../../model'; | ||
import { JobStatus, PrinterEvent, PrinterNotification, PrinterState, PrinterStatus, SocketAuth } from '../../model'; | ||
import { | ||
DisplayLayerProgressData, | ||
OctoprintFilament, | ||
|
@@ -25,7 +25,7 @@ export class OctoPrintSocketService implements SocketService { | |
|
||
private printerStatusSubject: Subject<PrinterStatus>; | ||
private jobStatusSubject: Subject<JobStatus>; | ||
private eventSubject: Subject<PrinterEvent>; | ||
private eventSubject: Subject<PrinterEvent | PrinterNotification>; | ||
|
||
private printerStatus: PrinterStatus; | ||
private jobStatus: JobStatus; | ||
|
@@ -39,7 +39,7 @@ export class OctoPrintSocketService implements SocketService { | |
) { | ||
this.printerStatusSubject = new ReplaySubject<PrinterStatus>(); | ||
this.jobStatusSubject = new Subject<JobStatus>(); | ||
this.eventSubject = new ReplaySubject<PrinterEvent>(); | ||
this.eventSubject = new ReplaySubject<PrinterEvent | PrinterNotification>(); | ||
} | ||
|
||
//==== SETUP & AUTH ====// | ||
|
@@ -116,6 +116,27 @@ export class OctoPrintSocketService implements SocketService { | |
this.socket.next(payload); | ||
} | ||
|
||
private handlePluginMessage(pluginMessage: OctoprintPluginMessage) { | ||
const plugins = [ | ||
{ | ||
check: (plugin: string) => plugin === 'DisplayLayerProgress-websocket-payload' | ||
&& this.configService.isDisplayLayerProgressEnabled(), | ||
handler: (data: unknown) => { | ||
this.extractFanSpeed(data as DisplayLayerProgressData); | ||
this.extractLayerHeight(data as DisplayLayerProgressData); | ||
}, | ||
}, | ||
{ | ||
check: (plugin: string) => ['action_command_prompt', 'action_command_notification'].includes(plugin), | ||
handler: (data: unknown) => this.eventSubject.next(data as PrinterNotification), | ||
}, | ||
]; | ||
|
||
plugins.forEach(plugin => | ||
plugin.check(pluginMessage.plugin.plugin) && plugin.handler(pluginMessage.plugin.data) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That definitely looks complicated (which isn't bad). Just curious: Will call this both check methods from the plugins array? Seems like a nice way to do this, since it's easily extendable 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok I think I understood it. Correct me if I'm wrong: This will go through top to bottom and call the check method with the plugin string. If the check message returns true it will call the handler in the same object, thus handle the message. Otherwise it will just fall-through until it either matches or reaches the end of the array. Really elegant way of handling this 👍 |
||
); | ||
} | ||
|
||
private setupSocket(resolve: () => void) { | ||
this.socket.subscribe(message => { | ||
if (Object.hasOwnProperty.bind(message)('current')) { | ||
|
@@ -124,14 +145,7 @@ export class OctoPrintSocketService implements SocketService { | |
} else if (Object.hasOwnProperty.bind(message)('event')) { | ||
this.extractPrinterEvent(message as OctoprintSocketEvent); | ||
} else if (Object.hasOwnProperty.bind(message)('plugin')) { | ||
const pluginMessage = message as OctoprintPluginMessage; | ||
if ( | ||
pluginMessage.plugin.plugin === 'DisplayLayerProgress-websocket-payload' && | ||
this.configService.isDisplayLayerProgressEnabled() | ||
) { | ||
this.extractFanSpeed(pluginMessage.plugin.data as DisplayLayerProgressData); | ||
this.extractLayerHeight(pluginMessage.plugin.data as DisplayLayerProgressData); | ||
} | ||
this.handlePluginMessage(message as OctoprintPluginMessage); | ||
} else if (Object.hasOwnProperty.bind(message)('reauth')) { | ||
this.systemService.getSessionKey().subscribe(socketAuth => this.authenticateSocket(socketAuth)); | ||
} else if (Object.hasOwnProperty.bind(message)('connected')) { | ||
|
@@ -303,7 +317,7 @@ export class OctoPrintSocketService implements SocketService { | |
return this.jobStatusSubject.pipe(startWith(this.jobStatus)); | ||
} | ||
|
||
public getEventSubscribable(): Observable<PrinterEvent> { | ||
public getEventSubscribable(): Observable<PrinterEvent | PrinterNotification> { | ||
return this.eventSubject; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe another way of doing this would be to check whether the received message is NOT of type
int
(which would indicate that it's an enum, thus PrinterEvent). It should work (haven't tested it though), but might be a bit cleaner.I wouldn't switch the logic around though, as the default should be handle PrinterEvent. Alternatively you could check whether the received message is of type
object
.