Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions packages/uhk-agent/src/services/device.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -358,13 +358,25 @@ export class DeviceService {
this.logService.misc('[DeviceService] load user configuration');

let response: ConfigurationReply;
let progress = 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please move this variable initialisation inside the sendProgress function. If I see weel we don't use it outside


try {
await this.stopPollUhkDevice();

const sendProgress = (percent: number) => {
progress = Math.max(progress, Math.min(100, percent));
event.sender.send(IpcEvents.device.loadConfigurationProgress, progress);
};

sendProgress(0);
await this.operations.waitUntilKeyboardBusy();
const result = await this.operations.loadConfigurations();
sendProgress(3);
const result = await this.operations.loadConfigurations((percent) => {
sendProgress(3 + Math.round(percent * 0.82));
});
sendProgress(88);
const modules: HardwareModules = await this.getHardwareModules(false);
sendProgress(95);

const hardwareConfig = getHardwareConfigFromDeviceResponse(result.hardwareConfiguration);
const uniqueId = hardwareConfig.uniqueId;
Expand All @@ -385,6 +397,7 @@ export class DeviceService {
info: BackupUserConfigurationInfo.Unknown
}
};
sendProgress(100);
} catch (error) {
response = {
success: false,
Expand Down Expand Up @@ -1298,18 +1311,32 @@ export class DeviceService {

try {
await this.stopPollUhkDevice();

let progress = 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please move this variable inside the `sendProgress. If I see well we don't use it ouside of the function

const sendProgress = (percent: number) => {
progress = Math.max(progress, Math.min(100, percent));
event.sender.send(IpcEvents.device.saveUserConfigurationProgress, progress);
};

sendProgress(0);
await backupUserConfiguration(data);
sendProgress(1);

this.logService.config('[DeviceService] User configuration will be saved', data.configuration);
const buffer = mapObjectToUserConfigBinaryBuffer(data.configuration);
await this.operations.saveUserConfiguration(buffer);
await this.operations.saveUserConfiguration(buffer, (percent) => {
sendProgress(1 + Math.round(percent * 0.94));
});

this._checkStatusBuffer = true;

if (data.saveInHistory) {
sendProgress(97);
await saveUserConfigHistoryAsync(buffer, data.deviceId, data.uniqueId);
await this.loadUserConfigFromHistory(event);
}

sendProgress(100);
response.success = true;
} catch (error) {
this.logService.error('[DeviceService] Transferring error', error);
Expand Down
2 changes: 2 additions & 0 deletions packages/uhk-common/src/util/ipcEvents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,10 @@ export class Device {
public static readonly setPrivilegeOnLinuxReply = 'set-privilege-on-linux-reply';
public static readonly deviceConnectionStateChanged = 'device-connection-state-changed';
public static readonly saveUserConfiguration = 'device-save-user-configuration';
public static readonly saveUserConfigurationProgress = 'device-save-user-configuration-progress';
public static readonly saveUserConfigurationReply = 'device-save-user-configuration-reply';
public static readonly loadConfigurations = 'device-load-configuration';
public static readonly loadConfigurationProgress = 'device-load-configuration-progress';
public static readonly loadConfigurationReply = 'device-load-configuration-reply';
public static readonly updateFirmware = 'device-update-firmware';
public static readonly updateFirmwareJson = 'device-update-firmware-json';
Expand Down
73 changes: 64 additions & 9 deletions packages/uhk-usb/src/uhk-operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -308,11 +308,36 @@ export class UhkOperations {
* Return with the actual UserConfiguration from UHK Device
* @returns {Promise<Buffer>}
*/
public async loadConfigurations(): Promise<LoadConfigurationsResult> {
public async loadConfigurations(onProgress?: (percent: number) => void): Promise<LoadConfigurationsResult> {
try {
await this.waitUntilKeyboardBusy();
const userConfiguration = await this.loadConfiguration(ConfigBufferId.validatedUserConfig);
const hardwareConfiguration = await this.loadConfiguration(ConfigBufferId.hardwareConfig);
onProgress?.(0);

const configSizes = await this.getConfigSizesFromKeyboard();
let userConfigSize = configSizes.userConfig;
const hardwareConfigSize = configSizes.hardwareConfig;

const reportTransferProgress = (userOffset: number, hardwareOffset: number) => {
const totalBytes = Math.max(userConfigSize + hardwareConfigSize, 1);
const transferredBytes = userOffset + hardwareOffset;

onProgress?.(Math.round(Math.min(transferredBytes / totalBytes, 1) * 100));
};

const userConfiguration = await this.loadConfiguration(
ConfigBufferId.validatedUserConfig,
(offset, configSize) => {
userConfigSize = configSize;
reportTransferProgress(offset, 0);
}
);
const hardwareConfiguration = await this.loadConfiguration(
ConfigBufferId.hardwareConfig,
(offset) => {
reportTransferProgress(userConfigSize, offset);
}
);
reportTransferProgress(userConfigSize, hardwareConfigSize);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need this last progress reporting?


return {
userConfiguration: JSON.stringify(convertBufferToIntArray(userConfiguration)),
Expand All @@ -327,7 +352,10 @@ export class UhkOperations {
* Return with the actual user / hardware fonfiguration from UHK Device
* @returns {Promise<Buffer>}
*/
public async loadConfiguration(configBufferId: ConfigBufferId): Promise<Buffer> {
public async loadConfiguration(
configBufferId: ConfigBufferId,
onProgress?: (offset: number, configSize: number) => void
): Promise<Buffer> {
const configBufferIdToName = ['HardwareConfig', 'StagingUserConfig', 'ValidatedUserConfig'];
const configName = configBufferIdToName[configBufferId];

Expand Down Expand Up @@ -360,6 +388,8 @@ export class UhkOperations {
configSize = originalConfigSize;
}
}

onProgress?.(offset, configSize);
}

return configBuffer;
Expand Down Expand Up @@ -393,8 +423,15 @@ export class UhkOperations {
};
}

public async saveUserConfiguration(buffer: Buffer): Promise<void> {
public async saveUserConfiguration(buffer: Buffer, onProgress?: (percent: number) => void): Promise<void> {
let lastProgress = 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please move this variable initialisation inside the reportProgress

const reportProgress = (percent: number) => {
lastProgress = Math.max(lastProgress, Math.min(100, Math.round(percent)));
onProgress?.(lastProgress);
};

try {
reportProgress(0);
this.logService.usbOps('[DeviceOperation] USB[T]: Write user configuration to keyboard');
let shouldRecalculateLength = false;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down Expand Up @@ -423,11 +460,20 @@ export class UhkOperations {

const resultBuffer = new UhkBuffer(UHK_EEPROM_SIZE)
userConfiguration.toBinary(resultBuffer)
await this.sendConfigToKeyboard(resultBuffer.getBufferContent(), true);
const configBuffer = resultBuffer.getBufferContent();

reportProgress(2);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think worth to the 2 into dedicated variable. If the value will change the developer don't have to remember to modify this value int reportProgress(2 + bytesSent / totalBytes * 83); line too

await this.sendConfigToKeyboard(configBuffer, true, (bytesSent, totalBytes) => {
reportProgress(2 + bytesSent / totalBytes * 83);
});
reportProgress(86);
await this.applyConfiguration();
reportProgress(90);
this.logService.usbOps('[DeviceOperation] USB[T]: Write user configuration to EEPROM');
await this.writeConfigToEeprom(ConfigBufferId.validatedUserConfig);
reportProgress(94);
await this.waitUntilKeyboardBusy();
reportProgress(96);
} catch (error) {
this.logService.error('[DeviceOperation] Transferring error', error);
throw error;
Expand Down Expand Up @@ -916,14 +962,23 @@ export class UhkOperations {
* @returns {Promise<void>}
* @private
*/
private async sendConfigToKeyboard(buffer: Buffer, isUserConfiguration): Promise<void> {
private async sendConfigToKeyboard(
buffer: Buffer,
isUserConfiguration,
onProgress?: (bytesSent: number, totalBytes: number) => void
): Promise<void> {
const command = isUserConfiguration
? UsbCommand.WriteStagingUserConfig
: UsbCommand.WriteHardwareConfig;

const fragments = getTransferBuffers(command, buffer);
for (const fragment of fragments) {
await this.device.write(fragment);
for (let i = 0; i < fragments.length; i++) {
await this.device.write(fragments[i]);
const bytesSent = Math.min(
buffer.length,
Math.round((i + 1) / fragments.length * buffer.length)
);
onProgress?.(bytesSent, buffer.length);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we could follow the other progress pattern when the callback called only with the percent variable. And we don't really need to calculate with the buffer.length . For the percent calculation enough the processed number of fragments / all fragments

}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
<button class="btn btn-primary"
<button class="btn btn-primary progress-button"
(click)="onClicked()"
[class.progress-button--active]="state.showProgress"
[disabled]="state.showProgress">
<fa-icon *ngIf="state.showProgress"
[icon]="faSpinner"
animation="spin"></fa-icon>
{{state.text}}
<span class="progress-button__fill"
role="progressbar"
[attr.aria-valuenow]="state.progressPercent ?? 0"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please set the default value in the ngrx state and don't use the nullish coalescing operator (??).

aria-valuemin="0"
aria-valuemax="100"
[style.width.%]="state.showProgress ? (state.progressPercent ?? 0) : 0">
</span>
<span class="progress-button__content">
{{state.text}}
</span>
</button>
Original file line number Diff line number Diff line change
@@ -1,3 +1,23 @@
button {
min-width: 150px;
}

.progress-button {
position: relative;
overflow: hidden;
}

.progress-button__fill {
position: absolute;
top: 0;
left: 0;
bottom: 0;
background-color: var(--color-loading-progress-bar-fill);
opacity: 0.35;
transition: width 200ms ease-out;
}

.progress-button__content {
position: relative;
z-index: 1;
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { ChangeDetectionStrategy, Component, EventEmitter, Input, Output } from '@angular/core';
import { Action } from '@ngrx/store';
import { faSpinner } from '@fortawesome/free-solid-svg-icons';

import { ProgressButtonState, initProgressButtonState } from '../../store/reducers/progress-button-state';

Expand All @@ -15,8 +14,6 @@ export class ProgressButtonComponent {
@Input() state: ProgressButtonState = initProgressButtonState;
@Output() clicked: EventEmitter<Action> = new EventEmitter<Action>();

faSpinner = faSpinner;
Comment thread
ert78gb marked this conversation as resolved.

onClicked() {
this.clicked.emit(this.state.action);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,22 @@
<div class="uhk-message-wrapper">
<uhk-agent-icon *ngIf="showLogo"
class="agent-logo"
[ngClass]="{'spin-logo': rotateLogo}">
</uhk-agent-icon>
<div class="messages">
<h1> {{ header }} </h1>
<h2 *ngIf="!smallText"> {{ subtitle }} </h2>
<div *ngIf="smallText"> {{ subtitle }} </div>
<p *ngIf="description"> {{ description }} </p>
<div class="uhk-message-content" [class.with-progress-bar]="showProgressBar">
<div class="uhk-message-main">
<uhk-agent-icon *ngIf="showLogo"
class="agent-logo"
[ngClass]="{'spin-logo': rotateLogo}">
</uhk-agent-icon>
<div class="messages">
<h1> {{ header }} </h1>
<h2 *ngIf="!smallText"> {{ subtitle }} </h2>
<div *ngIf="smallText"> {{ subtitle }} </div>
<p *ngIf="description"> {{ description }} </p>
</div>
</div>
<div *ngIf="showProgressBar" class="loading-progress-bar-wrapper">
<progress class="loading-progress-bar"
[value]="progressPercent"
max="100">
</progress>
</div>
</div>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,63 @@
justify-content: center;
}

.uhk-message-content {
display: flex;
flex-direction: column;
align-items: stretch;
}

.uhk-message-main {
display: flex;
align-items: center;
}

.agent-logo {
margin: 1.25em;
height: 8em;
width: 8em;
}

.with-progress-bar .agent-logo {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need this special behavior? Does not enough modify the .agent-logo selector

margin-bottom: 1.5em;
}

.loading-progress-bar-wrapper {
width: 100%;
padding: 0 1.25em;
}

.loading-progress-bar {
display: block;
width: 100%;
height: 1rem;
appearance: none;
border: none;
border-radius: 0.375rem;
overflow: hidden;
background-color: var(--color-loading-progress-bar-bg);

&::-webkit-progress-inner-element {
border: none;
}

&::-webkit-progress-bar {
background-color: var(--color-loading-progress-bar-bg);
}

&::-webkit-progress-value {
background-color: var(--color-loading-progress-bar-fill);
border-radius: 0;
transition: inline-size 200ms ease-out;
}

&::-moz-progress-bar {
background-color: var(--color-loading-progress-bar-fill);
border-radius: 0;
transition: inline-size 200ms ease-out;
}
}

.message {
display: flex;
flex-direction: column;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,7 @@ export class UhkMessageComponent {
@Input() subtitle: string;
@Input() rotateLogo = false;
@Input() showLogo = false;
@Input() showProgressBar = false;
@Input() progressPercent = 0;
@Input() smallText = false;
}
12 changes: 10 additions & 2 deletions packages/uhk-web/src/app/pages/loading-page/loading-device.page.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { Component } from '@angular/core';
import { Store } from '@ngrx/store';
import { Observable } from 'rxjs';

import { AppState, getConfigurationLoadingProgress } from '../../store';

@Component({
selector: 'loading-device',
Expand All @@ -7,11 +11,15 @@ import { Component } from '@angular/core';
<uhk-message header="Loading keyboard configuration..."
subtitle="Hang tight!"
[showLogo]="true"
[rotateLogo]="true"></uhk-message>
[rotateLogo]="true"
[showProgressBar]="true"
[progressPercent]="(progressPercent$ | async) ?? 0"></uhk-message>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please set the default value at state or selector level. Try to avoid from ?? operator

`,
})
export class LoadingDevicePageComponent {
progressPercent$: Observable<number>;

constructor() {
constructor(store: Store<AppState>) {
this.progressPercent$ = store.select(getConfigurationLoadingProgress);
}
}
Loading