Skip to content
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

Change how external QR code scanning works #19743

Merged
merged 5 commits into from
Feb 14, 2024
Merged

Conversation

balloob
Copy link
Member

@balloob balloob commented Feb 8, 2024

Breaking change

Proposed change

Based on feedback from Bram, change the way the external QR code scanner is integrated.

New format means the QR code scanner lifecycle is now fully controlled by the frontend.

Example interaction:

  1. frontend: Open QR code scanner (send bar_code/scan message)
  2. app: Opens the QR code scanner and start scanning
  3. app: Find result (send bar_code/scan_result message)
  4. frontend: Check if the result is ok. This time it's not so ignores result. Notify user by sending bar_code/notify message.
  5. app: Find result (send bar_code/scan_result message)
  6. frontend: Approves result. Tell app to close QR code scanner (send bar_code/close message)

If the user aborts the native QR code scanner, the app sends bar_code/aborted with a payload containing a reason: "canceled" | "alternative_options".

To test that this model works I have created an example consumer for the frontend. The only thing that is missing is listening to the external bus messaging:

import { HomeAssistant } from "../types";
import { EMIncomingMessageBarCodeScanResult } from "./external_messaging";

export class BarCodeError extends Error {
  public reason: string;

  constructor(reason: string) {
    super("Bar code scanning aborted");
    this.reason = reason;
  }
}

export const scanExternalBarCode = async (
  hass: HomeAssistant,
  title: string,
  description: string,
  isAcceptedBarCode: (format: string, barcode: string) => Promise<boolean>,
  alternativeOptionLabel?: string
): Promise<EMIncomingMessageBarCodeScanResult["payload"]> => {
  if (!hass.auth.external || !hass.auth.external.config.hasBarCodeScanner) {
    throw new Error("No external app bar code scanner available");
  }

  hass.auth.external.fireMessage({
    type: "bar_code/scan",
    title,
    description,
    alternative_option_label: alternativeOptionLabel,
  });

  // This class does not exist.
  const externalQRCodeMessages = new ExternalQRCodeMessages();

  const seen = new Set<string>();

  try {
    for await (const message of externalQRCodeMessages.listen()) {
      if (message.type === "bar_code/aborted") {
        throw new BarCodeError(message.payload.reason);
      }

      if (message.type === "bar_code/scan_result") {
        const key = `${message.payload.format}/${message.payload.code}`;

        // Prevent duplicate scans
        if (seen.has(key)) {
          continue;
        }

        if (
          await isAcceptedBarCode(message.payload.format, message.payload.code)
        ) {
          return message.payload;
        }

        hass.auth.external.fireMessage({
          type: "bar_code/notify",
          message: "Invalid bar code",
        });
        seen.add(key);
        continue;
      }

      // eslint-disable-next-line no-console
      console.warn("Unexpected bar code message", message);
    }

    throw new BarCodeError("stream_stopped");
  } finally {
    hass.auth.external.fireMessage({
      type: "bar_code/close",
    });
    externalQRCodeMessages.tearDown();
  }
};

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue or discussion:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

bramkragten
bramkragten previously approved these changes Feb 8, 2024
bramkragten
bramkragten previously approved these changes Feb 8, 2024
@balloob balloob marked this pull request as draft February 8, 2024 17:34
@balloob
Copy link
Member Author

balloob commented Feb 8, 2024

Marking as draft to add:

  • close message for the app
  • show toast message for the app ?

@balloob balloob changed the title Expand QR code scanning external bus Change how external QR code scanning works Feb 9, 2024
@balloob
Copy link
Member Author

balloob commented Feb 9, 2024

Updated the code with new commands.

@balloob balloob marked this pull request as ready for review February 9, 2024 15:25
@bgoncal
Copy link
Member

bgoncal commented Feb 12, 2024

When the app communicates "aborted" should it dismiss right away or wait for frontend to ask it to dismiss? (in case when the aborted comes from alternative options, close button will always close disregarding frontend)

@balloob
Copy link
Member Author

balloob commented Feb 12, 2024

I think that when the app sends an aborted, it's ok to already close the QR code scanner.

@bramkragten bramkragten merged commit 553230c into dev Feb 14, 2024
13 checks passed
@bramkragten bramkragten deleted the expand-qr-scanning branch February 14, 2024 08:56
@github-actions github-actions bot locked and limited conversation to collaborators Feb 13, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants