|
1 | 1 | import { ClientDuplexStream } from '@grpc/grpc-js'; |
2 | 2 | import { Disposable, Emitter, ILogger } from '@theia/core'; |
3 | 3 | import { inject, named, postConstruct } from '@theia/core/shared/inversify'; |
| 4 | +import { diff, Operation } from 'just-diff'; |
4 | 5 | import { Board, Port, Status, Monitor } from '../common/protocol'; |
5 | 6 | import { |
6 | 7 | EnumerateMonitorPortSettingsRequest, |
@@ -80,6 +81,15 @@ export class MonitorService extends CoreClientAware implements Disposable { |
80 | 81 | private readonly port: Port; |
81 | 82 | private readonly monitorID: string; |
82 | 83 |
|
| 84 | + /** |
| 85 | + * The lightweight representation of the port configuration currently in use for the running monitor. |
| 86 | + * IDE2 stores this object after starting the monitor. On every monitor settings change request, IDE2 compares |
| 87 | + * the current config with the new settings, and only sends the diff as the new config to overcome https://github.com/arduino/arduino-ide/issues/375. |
| 88 | + */ |
| 89 | + private currentPortConfigSnapshot: |
| 90 | + | MonitorPortConfiguration.AsObject |
| 91 | + | undefined; |
| 92 | + |
83 | 93 | constructor( |
84 | 94 | @inject(MonitorServiceFactoryOptions) options: MonitorServiceFactoryOptions |
85 | 95 | ) { |
@@ -211,6 +221,16 @@ export class MonitorService extends CoreClientAware implements Disposable { |
211 | 221 | monitorRequest |
212 | 222 | ); |
213 | 223 | if (wroteToStreamSuccessfully) { |
| 224 | + // Only store the config, if the monitor has successfully started. |
| 225 | + this.currentPortConfigSnapshot = MonitorPortConfiguration.toObject( |
| 226 | + false, |
| 227 | + config |
| 228 | + ); |
| 229 | + this.logger.info( |
| 230 | + `Using port configuration for ${this.port.protocol}:${ |
| 231 | + this.port.address |
| 232 | + }: ${JSON.stringify(this.currentPortConfigSnapshot)}` |
| 233 | + ); |
214 | 234 | this.startMessagesHandlers(); |
215 | 235 | this.logger.info( |
216 | 236 | `started monitor to ${this.port?.address} using ${this.port?.protocol}` |
@@ -518,16 +538,120 @@ export class MonitorService extends CoreClientAware implements Disposable { |
518 | 538 | if (!this.duplex) { |
519 | 539 | return Status.NOT_CONNECTED; |
520 | 540 | } |
| 541 | + |
| 542 | + const diffConfig = this.maybeUpdatePortConfigSnapshot(config); |
| 543 | + if (!diffConfig) { |
| 544 | + this.logger.info( |
| 545 | + `No port configuration changes have been detected. No need to send configure commands to the running monitor ${this.port.protocol}:${this.port.address}.` |
| 546 | + ); |
| 547 | + return Status.OK; |
| 548 | + } |
| 549 | + |
521 | 550 | const coreClient = await this.coreClient; |
522 | 551 | const { instance } = coreClient; |
523 | 552 |
|
| 553 | + this.logger.info( |
| 554 | + `Sending monitor request with new port configuration: ${JSON.stringify( |
| 555 | + MonitorPortConfiguration.toObject(false, diffConfig) |
| 556 | + )}` |
| 557 | + ); |
524 | 558 | const req = new MonitorRequest(); |
525 | 559 | req.setInstance(instance); |
526 | | - req.setPortConfiguration(config); |
| 560 | + req.setPortConfiguration(diffConfig); |
527 | 561 | this.duplex.write(req); |
528 | 562 | return Status.OK; |
529 | 563 | } |
530 | 564 |
|
| 565 | + /** |
| 566 | + * Function to calculate a diff between the `otherPortConfig` argument and the `currentPortConfigSnapshot`. |
| 567 | + * |
| 568 | + * If the current config snapshot and the snapshot derived from `otherPortConfig` are the same, no snapshot update happens, |
| 569 | + * and the function returns with undefined. Otherwise, the current snapshot config value will be updated from the snapshot |
| 570 | + * derived from the `otherPortConfig` argument, and this function returns with a `MonitorPortConfiguration` instance |
| 571 | + * representing only the difference between the two snapshot configs to avoid sending unnecessary monitor to configure commands to the CLI. |
| 572 | + * See [#1703 (comment)](https://github.com/arduino/arduino-ide/pull/1703#issuecomment-1327913005) for more details. |
| 573 | + */ |
| 574 | + private maybeUpdatePortConfigSnapshot( |
| 575 | + otherPortConfig: MonitorPortConfiguration |
| 576 | + ): MonitorPortConfiguration | undefined { |
| 577 | + const otherPortConfigSnapshot = MonitorPortConfiguration.toObject( |
| 578 | + false, |
| 579 | + otherPortConfig |
| 580 | + ); |
| 581 | + if (!this.currentPortConfigSnapshot) { |
| 582 | + throw new Error( |
| 583 | + `The current port configuration object was undefined when tried to merge in ${JSON.stringify( |
| 584 | + otherPortConfigSnapshot |
| 585 | + )}.` |
| 586 | + ); |
| 587 | + } |
| 588 | + |
| 589 | + const snapshotDiff = diff( |
| 590 | + this.currentPortConfigSnapshot, |
| 591 | + otherPortConfigSnapshot |
| 592 | + ); |
| 593 | + if (!snapshotDiff.length) { |
| 594 | + return undefined; |
| 595 | + } |
| 596 | + |
| 597 | + const diffConfig = snapshotDiff.reduce((acc, curr) => { |
| 598 | + if (!this.isValidMonitorPortSettingChange(curr)) { |
| 599 | + throw new Error( |
| 600 | + `Expected only 'replace' operation: a 'value' change in the 'settingsList'. Calculated diff a ${JSON.stringify( |
| 601 | + snapshotDiff |
| 602 | + )} between ${JSON.stringify( |
| 603 | + this.currentPortConfigSnapshot |
| 604 | + )} and ${JSON.stringify( |
| 605 | + otherPortConfigSnapshot |
| 606 | + )} snapshots. Current JSON-patch entry was ${JSON.stringify(curr)}.` |
| 607 | + ); |
| 608 | + } |
| 609 | + const { path, value } = curr; |
| 610 | + const [, index] = path; |
| 611 | + if (!this.currentPortConfigSnapshot?.settingsList) { |
| 612 | + throw new Error( |
| 613 | + `'settingsList' is missing from current port config snapshot: ${JSON.stringify( |
| 614 | + this.currentPortConfigSnapshot |
| 615 | + )}` |
| 616 | + ); |
| 617 | + } |
| 618 | + const changedSetting = this.currentPortConfigSnapshot.settingsList[index]; |
| 619 | + const setting = new MonitorPortSetting(); |
| 620 | + setting.setValue(value); |
| 621 | + setting.setSettingId(changedSetting.settingId); |
| 622 | + acc.addSettings(setting); |
| 623 | + return acc; |
| 624 | + }, new MonitorPortConfiguration()); |
| 625 | + |
| 626 | + this.currentPortConfigSnapshot = otherPortConfigSnapshot; |
| 627 | + this.logger.info( |
| 628 | + `Updated the port configuration for ${this.port.protocol}:${ |
| 629 | + this.port.address |
| 630 | + }: ${JSON.stringify(this.currentPortConfigSnapshot)}` |
| 631 | + ); |
| 632 | + return diffConfig; |
| 633 | + } |
| 634 | + |
| 635 | + private isValidMonitorPortSettingChange(entry: { |
| 636 | + op: Operation; |
| 637 | + path: (string | number)[]; |
| 638 | + value: unknown; |
| 639 | + }): entry is { |
| 640 | + op: 'replace'; |
| 641 | + path: ['settingsList', number, string]; |
| 642 | + value: string; |
| 643 | + } { |
| 644 | + const { op, path, value } = entry; |
| 645 | + return ( |
| 646 | + op === 'replace' && |
| 647 | + path.length === 3 && |
| 648 | + path[0] === 'settingsList' && |
| 649 | + typeof path[1] === 'number' && |
| 650 | + path[2] === 'value' && |
| 651 | + typeof value === 'string' |
| 652 | + ); |
| 653 | + } |
| 654 | + |
531 | 655 | /** |
532 | 656 | * Starts the necessary handlers to send and receive |
533 | 657 | * messages to and from the frontend and the running monitor |
|
0 commit comments