[APP][Pro] Casambi Controller - Lighting control for the modern world

Hèb je hier iets aan?

Here are several issues and potential improvements in the provided code snippet:

  1. getSocket Method Empty Implementation:
    The getSocket method does not have any implementation, which could be intentional (e.g., for overriding in subclasses) but if not, it should either have a meaningful implementation or be documented as to why it’s empty.

  2. Reconnection Logic Could Be Improved:
    The reconnect method clears a timer and attempts to delete a socket from an unspecified this.sockets object based on network properties. However, there’s no check to see if the socket actually exists before attempting deletion. Additionally, the reconnection is attempted immediately after deletion without any delay or failure handling, which might not be effective in all network conditions.

  3. Authentication Check Simplification:
    The isAuthenticated method returns a boolean based on the existence of this.networks. This is overly simplistic as it does not actually check if the session is still valid. The commented part (// TODO: && Date.now() < this.networks.expires_at) should be implemented to check for session expiration.

  4. Login Method Error Handling and Redundancy:
    The login method has an initial check if (this.isLoggingIn), which is good for preventing simultaneous login attempts. However, the logic to handle this situation involves a mere 100ms delay before retrying, which may not be sufficient or necessary. Consider implementing a more robust method for handling simultaneous logins, such as using event-driven techniques or a longer, possibly exponential, backoff strategy.

  5. Exception Handling in Login:
    When the login fails (if (!response.ok)), the method logs and then throws an error. This is generally fine, but the thrown error should also include the HTTP status code for better debugging and error handling.

  6. Use of Magic Strings and Numbers:
    There are several “magic” strings and numbers (like the URL path and the 100ms timeout), which should ideally be defined as constants at the top of the class or module for easier maintenance and configuration.

  7. Headers Manipulation:
    The delete this.headers['X-Casambi-Session']; operation directly manipulates headers without any check. It would be safer and clearer to manage headers in a more controlled manner, especially considering the role of headers in authentication processes.

By addressing these issues, the code can be made more robust, maintainable, and clear in its intent and handling of network operations and authentication.

Here’s a rewritten version of the provided code, incorporating better practices, including error handling, use of constants for configuration values, and more structured login control to handle concurrency effectively:

class NetworkManager {
    private static readonly BASE_URL = 'https://api.example.com';
    private static readonly AUTHENTICATE_URL = `${NetworkManager.BASE_URL}/networks/session`;
    private static readonly RECONNECT_DELAY_MS = 1000; // 1 second delay for reconnection

    private isLoggingIn = false;
    private sockets: { [key: string]: WebSocket } = {};
    private networks: NetworkList | null = null;
    private headers: { [key: string]: string } = {};

    constructor(private username: string, private password: string) {}

    protected getSocket(network: Network): WebSocket {
        const key = `${network.id}-${network.sessionId}`;
        let socket = this.sockets[key];
        if (!socket) {
            socket = new WebSocket(NetworkManager.BASE_URL);
            this.sockets[key] = socket;
        }
        return socket;
    }

    protected reconnect(network: Network) {
        const key = `${network.id}-${network.sessionId}`;
        const existingTimer = this.sockets[key];
        if (existingTimer) {
            clearInterval(existingTimer as unknown as NodeJS.Timer);
            delete this.sockets[key];
            setTimeout(() => this.getSocket(network), NetworkManager.RECONNECT_DELAY_MS);
        }
    }

    protected isAuthenticated(): boolean {
        return this.networks !== null && Date.now() < (this.networks.expires_at ?? Number.MAX_VALUE);
    }

    protected async login(): Promise<NetworkList> {
        if (this.isLoggingIn) {
            await new Promise(resolve => setTimeout(resolve, NetworkManager.RECONNECT_DELAY_MS));
            return this.getNetworks();
        }

        this.isLoggingIn = true;
        delete this.headers['X-Casambi-Session'];

        try {
            const response = await fetch(NetworkManager.AUTHENTICATE_URL, {
                method: 'POST',
                body: JSON.stringify({ email: this.username, password: this.password }),
                headers: this.headers as HeadersInit,
            });

            if (!response.ok) {
                const errorText = await response.text();
                console.error(`Authentication failed for ${this.username}: ${errorText}`);
                throw new Error(`Authentication failed with status ${response.status}: ${errorText}`);
            }

            this.networks = await response.json() as NetworkList;
            console.log(`Authentication succeeded for ${this.username}`);
            return this.networks;
        } finally {
            this.isLoggingIn = false;
        }
    }

    private getNetworks(): NetworkList {
        if (!this.networks) {
            throw new Error("No network information available.");
        }
        return this.networks;
    }
}

Key Changes and Improvements:

  • Constant Usage: Constants are used for URLs and time delays to make configuration easier and the code cleaner.
  • Error Handling in Login: The login function now includes better handling with try-finally to ensure this.isLoggingIn is reset properly.
  • Reconnect Delay: A constant is used for the reconnection delay, and the reconnection logic checks if the socket exists before attempting to reconnect.
  • Session Validation: The isAuthenticated method now correctly validates the session based on an expiration time.
  • Networks Getter: A private method getNetworks safely retrieves network information, throwing an error if it’s not available.

This rewrite aims to enhance readability, maintainability, and error handling in the code.

Dit is de volledige ChatGPT versie :smiley:

To address the identified issues and improve the design of the Client class, I will refactor the code to enhance modularity, readability, and error handling. Here’s an improved version of the TypeScript class with some additional comments and adjustments:

import fetch, { HeadersInit } from 'node-fetch';
import WebSocket from 'ws';

const BASE_URL = 'https://door.casambi.com/v1';
const RECONNECT_INTERVAL_MS = 240000; // 4 minutes
const LOGIN_RETRY_DELAY_MS = 1000; // 1 second

interface Network {
    id: string;
    sessionId: string;
}

interface NetworkList {
    [id: string]: Network;
}

interface DeviceEventCallback {
    (data: any): void;
}

class WebSocketManager {
    private socket: WebSocket | null = null;
    private intervalId: NodeJS.Timeout | null = null;

    constructor(private token: string, private network: Network) {}

    connect() {
        const socketUrl = `wss://door.casambi.com/v1/bridge/${this.token}`;
        this.socket = new WebSocket(socketUrl);

        this.socket.on('open', () => {
            this.keepAlive();
        });

        this.socket.on('close', () => {
            this.reconnect();
        });

        this.socket.on('error', error => {
            console.error('WebSocket Error:', error);
            this.reconnect();
        });
    }

    send(data: string) {
        if (this.socket?.readyState === WebSocket.OPEN) {
            this.socket.send(data);
        }
    }

    private keepAlive() {
        this.intervalId = setInterval(() => {
            if (this.socket?.readyState === WebSocket.OPEN) {
                const ping = JSON.stringify({ method: 'ping' });
                this.socket.send(ping);
            }
        }, RECONNECT_INTERVAL_MS);
    }

    private reconnect() {
        if (this.intervalId) {
            clearInterval(this.intervalId);
        }
        this.connect();
    }

    close() {
        if (this.intervalId) {
            clearInterval(this.intervalId);
        }
        if (this.socket) {
            this.socket.close();
        }
    }
}

class Client {
    private isLoggingIn = false;
    private networks?: NetworkList;
    private headers: HeadersInit = {
        'Content-Type': 'application/json',
        'X-Casambi-Key': '',
    };
    private webSocketManager?: WebSocketManager;

    constructor(private token: string, private username: string, private password: string) {
        this.headers['X-Casambi-Key'] = token;
    }

    async authenticate(): Promise<void> {
        if (this.isLoggingIn) {
            await new Promise(resolve => setTimeout(resolve, LOGIN_RETRY_DELAY_MS));
            return;
        }

        this.isLoggingIn = true;
        const url = `${BASE_URL}/networks/session`;
        const body = JSON.stringify({ email: this.username, password: this.password });

        const response = await fetch(url, {
            method: 'POST',
            headers: this.headers,
            body: body,
        });

        if (!response.ok) {
            const errorText = await response.text();
            throw new Error(`Authentication failed: ${errorText}`);
        }

        this.networks = await response.json() as NetworkList;
        this.isLoggingIn = false;
    }

    async getNetwork(networkId: string): Promise<Network> {
        if (!this.networks) {
            throw new Error('No networks available');
        }
        return this.networks[networkId];
    }

    connectToNetwork(networkId: string) {
        if (!this.networks) {
            throw new Error('Not authenticated or no networks available');
        }
        const network = this.networks[networkId];
        this.webSocketManager = new WebSocketManager(this.token, network);
        this.webSocketManager.connect();
    }

    disconnectFromNetwork() {
        if (this.webSocketManager) {
            this.webSocketManager.close();
        }
    }

    async testCredentials(): Promise<boolean> {
        try {
            await this.authenticate();
            return true;
        } catch {
            return false;
        }
    }
}

export default Client;

Key Changes Made:

  1. Modular Design: Separated WebSocket handling into its own class, WebSocketManager, to encapsulate WebSocket lifecycle management, including connection, keep-alive, and error handling.

  2. Error Handling: Improved throughout, particularly in network requests, with more precise error messages.

  3. Authentication Method: Renamed login to authenticate for clarity and adjusted to prevent multiple simultaneous login attempts more effectively.

  4. Connection Management: Methods connectToNetwork and disconnectFromNetwork manage network connections, leveraging the WebSocketManager.

  5. Configuration Constants: Introduced constants for timings and URLs for easier modification and clearer code.

This refactoring improves the overall structure, making the code more maintainable, secure, and easier to debug while

Nice one @Martijn_de_Koning I haven’t checked the code in detail yet, but from what I’ve scanned in it’s observations it might have found the cause and has provided a solution.

As soon as I have some time I will try to implement it

2 Likes

Any update Leon? :smiley: Niets werkt namelijk nu en dat is erg onhandig :sweat_smile:

Dear,

So I tried to login at my Casambi network but is says; The credentials are not right.
Does anyone else have the same problem?

I have exactly the same problem, can’t log in to Casambi cloud.

Best regards

Mattias Ulén

People read: [App][Pro] Casambi - #60 by Vesa_Vainio
Casambi noticed the app is flawed!

@Martijn_de_Koning proposed a solution with the help of ChatGPT, but I haven’t got the time to take a proper look at it yet and implement a fix accordingly.

I hope to have some time this week to update the app and see if this fixes the issue. To be updated

2 Likes

Thought my setup was broken, happy to see a fix is being worked on :slight_smile:

Dear Vesa Vainio
Thanks for your information. I am not sure, but is there no interest of casambi to build the homey connection by yourself, so that it will work properly for all of the users?
I just bought a casambi Cloud Gateway and hoped this would help to connect with homey.
I look forward to hear from you!
Karel

Hello Karel,

Unfortunately, we at Casambi don’t have the resources to contribute development work for the Homey app. We are not familiar with the Homey platform, or developing apps for the Homey platform at all. We don’t have any Homey devices and thus no possibility to test the app ourselves.

This is also how we usually work. We have hundreds of partners creating and selling Casambi-enabled products. The partners do their own development, and we provide help with the relevant Casambi interfaces, and also develop features on our systems when necessary and feasible.

Our general API documentation is available at https://developer.casambi.com/ and we can try to help by e.g. answering questions about the documentation or the API functionality.

I’m also having a direct discussion with one of the community members about the possibilities for testing a modified version of the app.

Best regards,
Vesa Vainio

Thank you very much for your quick answer. As I am “only” a user and not a programmer I would be very happy to use my Casambi Installation (35 products) with homey

The app has been modified. The code changes have been communicated to Casambi. Now we wait until the API is made available again.

1 Like