Problem implementing garage door - Home app doesn't show correct status

[jack-bradshaw]

I'm implementing the GarageDoor interface, however I'm finding that the iOS Home app doesn't display the correct door state. Here's my implementation:

public class RemoteGarageDoor implements GarageDoor {
    /**
     * A connection to the door which allows data to be sent and received.
     */
    private final GarageDoorConnection connection;

    private final int id;

    private final String label;

    private final String manufacturer;

    private final String model;

    private final String serialNumber;

    /**
     * Futures passed to {@link #getCurrentDoorState()} which must be completed next time the door 
     * status is updated.
     */
    private final Set<CompletableFuture<DoorState>> getDoorStateFutures =
            ConcurrentHashMap.newKeySet();

    /**
     * Futures passed to {@link #getObstructionDetected()} which must be completed next time the 
     * door status is updated.
     */
    private final Set<CompletableFuture<Boolean>> getObstructionDetectedFutures =
            ConcurrentHashMap.newKeySet();

    /**
     * Future from the last call to {@link #setTargetDoorState(DoorState)} which must be 
     * completed when the door reaches the target state.
     */
    private CompletableFuture<Void> targetStateReachedFuture;

    private DoorState lastKnownDoorState = CLOSED;

    private boolean lastKnownObstructionDetected = false;

    private DoorState targetDoorState = CLOSED;

    /**
     * Whether or not the current target door state has been sent to the door.
     */
    private boolean targetHasBeenRequested = true;

    private HomekitCharacteristicChangeCallback doorStateCallback;

    private HomekitCharacteristicChangeCallback targetDoorStateCallback;

    private HomekitCharacteristicChangeCallback obstructionDetectedCallback;

    public RemoteGarageDoor(final GarageDoorConnection connection,
            final int id,
            final String label,
            final String manufacturer,
            final String model,
            final String serialNumber) {
        this.connection = checkNotNull(connection, "connection cannot be null.");
        this.id = id;
        this.label = checkNotNull(label, "label cannot be null.");
        this.manufacturer = checkNotNull(manufacturer, "manufacturer cannot be null.");
        this.model = checkNotNull(model, "model cannot be null.");
        this.serialNumber = checkNotNull(serialNumber, "serialNumber cannot be null.");

        // Use a background thread for looping
        Executors.newSingleThreadExecutor().execute(() -> {
            while (true) {
                try {
                    loop();
                } catch (final Exception e) {
                    Lumberjack.log(ERROR, "Error occurred in RemoteGarageDoor loop.", e);
                }
            }
        });
    }

    @Override
    public String getLabel() {
        return label;
    }

    @Override
    public String getManufacturer() {
        return manufacturer;
    }

    @Override
    public String getModel() {
        return model;
    }

    @Override
    public String getSerialNumber() {
        return serialNumber;
    }

    @Override
    public int getId() {
        return id;
    }

    @Override
    public void identify() {
        Lumberjack.log(WARNING, "identify() not implemented for RemoteGarageDoor.");
    }

    @Override
    public CompletableFuture<DoorState> getCurrentDoorState() {
        final CompletableFuture<DoorState> future = new CompletableFuture<>();

        // Save the future so that it can be completed by the looper
        getDoorStateFutures.add(future);

        return future;
    }

    @Override
    public CompletableFuture<DoorState> getTargetDoorState() {
        return CompletableFuture.completedFuture(targetDoorState);
    }

    @Override
    public CompletableFuture<Boolean> getObstructionDetected() {
        final CompletableFuture<Boolean> future = new CompletableFuture<>();

        // Save the future so that it can be completed by the looper
        getObstructionDetectedFutures.add(future);

        return future;
    }

    @Override
    public CompletableFuture<Void> setTargetDoorState(final DoorState targetDoorState) throws
            Exception {
        final CompletableFuture<Void> future = new CompletableFuture<>();

        if (this.targetDoorState != targetDoorState) {
            // Deliver callback if necessary
            if (targetDoorStateCallback != null) {
                targetDoorStateCallback.changed();
            }

            // Save the new target state
            this.targetDoorState = targetDoorState;

            // Save the future so it can be completed by the looper
            targetStateReachedFuture = future;

            // Indicate to the looper that the new target has not yet been requested
            targetHasBeenRequested = false;
        }

        return future;
    }

    @Override
    public void subscribeCurrentDoorState(final HomekitCharacteristicChangeCallback callback) {
        doorStateCallback = callback;
    }

    @Override
    public void subscribeTargetDoorState(final HomekitCharacteristicChangeCallback callback) {
        targetDoorStateCallback = callback;
    }

    @Override
    public void subscribeObstructionDetected(final HomekitCharacteristicChangeCallback callback) {
        obstructionDetectedCallback = callback;
    }

    @Override
    public void unsubscribeCurrentDoorState() {
        doorStateCallback = null;
    }

    @Override
    public void unsubscribeTargetDoorState() {
        targetDoorStateCallback = null;
    }

    @Override
    public void unsubscribeObstructionDetected() {
        obstructionDetectedCallback = null;
    }

    /**
     * Requests that the door open/close etc. or requests a status update from the door. The
     * former has priority.
     */
    private void loop() {
        if (targetDoorState != lastKnownDoorState && !targetHasBeenRequested) {
            requestDoorStateChange();
            targetHasBeenRequested = true;
        } else {
            final ResponsePacket response = connection.sendRequestAndAwaitResponse(
                    RequestPacket.newGetStatusUpdatedRequestPacket());

            // The door may not always be reachable.
            if (response != null) {
                processUpdate(response);
            }
        }
    }

    /**
     * Sends a request to the door to change state.
     */
    private void requestDoorStateChange() {
        //TODO implement confirmation return oacket, OK for now while using a wired connection
        if (targetDoorState == OPEN) {
            connection.sendRequest(RequestPacket.newOpenDoorRequestPacket());

        } else if (targetDoorState == CLOSED) {
            connection.sendRequest(RequestPacket.newCloseDoorRequestPacket());

        } else if (targetDoorState == STOPPED) {
            connection.sendRequest(RequestPacket.newStopDoorRequestPacket());

        } else {
            throw new RuntimeException("The target state is invalid.");
        }
    }

    /**
     * Processes a status update from the door. Pending futures are completed if appropriate and
     * the necessary callbacks are delivered.
     *
     * @param update
     *      the update from the door, not null
     */
    private void processUpdate(final ResponsePacket update) {
        // Copy the current status
        final DoorState oldState = lastKnownDoorState;
        final boolean oldObstructionDetected = lastKnownObstructionDetected;

        // Update the current status
        lastKnownDoorState = update.getDoorState();
        lastKnownObstructionDetected = update.getObstructionDetectedStatus();

        // If the target state has been reached, complete the relevant future
        if (targetDoorState == update.getDoorState() && targetStateReachedFuture != null) {
            targetStateReachedFuture.complete(null);
            targetStateReachedFuture = null; // Completed, so discard
        }

        // Complete futures of getDoorState()
        for (final CompletableFuture<DoorState> future : getDoorStateFutures) {
            future.complete(update.getDoorState());
            getDoorStateFutures.remove(future); // Completed, so discard
        }

        // Complete futures of getObstructionDetected()
        for (final CompletableFuture<Boolean> future : getObstructionDetectedFutures) {
            future.complete(update.getObstructionDetectedStatus());
            getObstructionDetectedFutures.remove(future); // Completed, so discard
        }

        // Deliver door state change callback if necessary
        if (oldState != update.getDoorState() && doorStateCallback != null) {
            doorStateCallback.changed();
        }

        // Deliver obstruction detected changed callback if necessary
        if (oldObstructionDetected != update.getObstructionDetectedStatus()
                && obstructionDetectedCallback != null) {
            obstructionDetectedCallback.changed();
        }
    }
}

Problems occur when the door state is changed. For example:

• The iOS home app shows the status of closed. This is expected and reflects the internal representation of the door state.
• Using the home app, a request to open the door is made.
• The mock door opens and my garage door implementation completes appropriate saved futures.
• New calls to getCurrentDoorState() result in the future being completed with a status of open.

Despite this, the home app continues to show "Opening..." as the door status.

My unit tests show that the futures are being completed as expected and the correct callbacks are being delivered. Also right now I'm mocking out the connection to the physical door, so there's no packet loss or other networking issues.

Any ideas?

[jack-bradshaw]

After further investigation, I've found that execution blocks when HomekitCharacteristicChangeCallback.changed() is called in void processUpdate(final ResponsePacket update). I mocked these callbacks in my unit tests which would explain why they passed but my integration tests don't. Is this blocking behaviour expected?

[TyrantFox]

@beowulfe Is there an expected response time from the characteristic requests? I'm trying to do something somewhat similar to Matthew, in which I want to delegate all the actual work to the microcontroller on the garage door, and have my HAP server send MQTT requests and correlate the responses.

Currently I make a CompletableFuture when the value is requested. Any subsequent requests while I'm waiting for a response are given the same instance of the CompletableFuture. Then when I get a response back over MQTT I notify the subscriber (if any), and then complete the future, satisfying all the pending requests.

However since the controller may be offline, I need to do something meaningful if I don't get a response back in a reasonable time, or the gets for will block for long periods.

Is this a suitable approach to take, and what would be reasonable timeout behavior? I think returning null generates errors in the framework, but perhaps a suitable "no response" behavior in iOS will still happen.

If you can answer these questions I'd be happy to write a bit of documentation on the wiki page, and update the demo, perhaps add something a bit more complicated than the light.

[ccutrer]

Are you still having problems with the latest main branch?

[jack-bradshaw]

I abandoned my project years ago.