Exception when creating AirQualitySensor with room2 and history set to true

[pdavid31]

As described in the title i get a ReferenceError when creating an AirQualitySensor with the attributes room2 and history set to true.

Versions

Node.js: 16.13.1 NPM: 8.3.0 HomeBridge: 1.3.8 mqttthing: 1.1.35

Config

{
    "accessory": "mqttthing",
    "type": "airQualitySensor",
    "name": "Büro BME680",
    "manufacturer": "Bosch",
    "model": "BME680",
    "url": "<BROKER_URL>",
    "username": "<BROKER_USER>",
    "password": "<BROKER PASSWORD>",
    "serviceNames": {
        "temperature": "Büro Temperatur",
        "humidity": "Büro Luftfeuchtigkeit"
    },
    "topics": {
        "getAirQuality": "tele/office/iaq",
        "getVOCDensity": "tele/office/voc",
        "getCarbonDioxideLevel": "tele/office/co2",
        "getCurrentTemperature": "tele/office/temperature",
        "getCurrentRelativeHumidity": "tele/office/humidity",
        "getStatusActive": "availability/office/monitor"
    },
    "history": true,
    "room2": true
}

Logs:

[20/12/2021, 05:34:17] [Büro BME680] Initializing mqttthing accessory...
[20/12/2021, 05:34:17] [Büro BME680] Exception while creating services: ReferenceError: tempSvc is not defined
[20/12/2021, 05:34:17] [Büro BME680] ReferenceError: tempSvc is not defined
    at configToServices (/usr/local/lib/node_modules/homebridge-mqttthing/index.js:3143:61)
    at createServices (/usr/local/lib/node_modules/homebridge-mqttthing/index.js:3387:24)
    at new makeThing (/usr/local/lib/node_modules/homebridge-mqttthing/index.js:3419:23)
    at /usr/local/lib/node_modules/homebridge/src/server.ts:350:50
    at Array.forEach (<anonymous>)
    at Server.loadAccessories (/usr/local/lib/node_modules/homebridge/src/server.ts:270:29)
    at Server.start (/usr/local/lib/node_modules/homebridge/src/server.ts:162:12)
[20/12/2021, 05:34:17] [Büro BME680] Accessory mqttthing returned empty set of services; not adding it to the bridge.

[pdavid31]

My JavaScript skills are a bit rusty, but as mentioned in the logs the problem seems to be in line 3413 of index.js, where it seems like tempSvc is not in scope.

[arachnetech]

Good spot! I don't use airQualitySensor myself (and it was kindly added by a contributor), but it looks like that line belongs around line 3131 instead. Let me take a close look...

[arachnetech]

I believe I've fixed the crash, but I don't know whether the functionality is correct. @dd4rk, perhaps you could take a look? There are three issues with temperature display units in Eve room2 support:

1. There was an existing characteristic_TemperatureDisplayUnits function already defined, which the new Eve-specific function clashed with. (I'd broken my eslint config, otherwise this would have been very clear!)
2. The new characteristic_TemperatureDisplayUnits function appears to define the wrong characteristic type and so generates warnings. I've disabled it for now and am using the original function, but I don't know whether this is correct for your room2 support?
3. As @pdavid31 spotted, the call at https://github.com/arachnetech/homebridge-mqttthing/blob/master/index.js#L3153 was passing the undefined tempSvc causing his crash. I've changed this to the historySvc in-scope there, but I don't know enough about Eve extensions to know whether that makes sense, or whether we should be calling this on the tempSvc above if room2 if set?

[D4rkCrypto]

Sorry for the errors I created. According to fakegato-history, the temperature sensor needs to implement the Characteristic.TemperatureDisplayUnits Characteristic set to Celsius when using eve room2.

I didn't notice there is a characteristic_TemperatureDisplayUnits function already. I have checked that this existing function is enough to use in this case.

Put This line after this line should fix this issue.

That is, delete characteristic_TemperatureDisplayUnits( historySvc ); and add characteristic_TemperatureDisplayUnits( tempSvc ); in the if statement of config.topics.getCurrentTemperature.

In addition, users who want to use room2 should make sure they use Celsius as the temperature display unit (default to Celsius if not specified) and pass value to getCurrentTemperature. An example config is,

        {
            "type": "airQualitySensor",
            "name": "Air Quality",
            "accessory": "mqttthing",
            "topics": {
                "getAirQuality": "homeBridge/airQualitySensor/AirQuality",
                "getVOCDensity": "homeBridge/airQualitySensor/VOCDensity",
                "getCurrentTemperature": "homeBridge/airQualitySensor/CurrentTemperature",
                "getCurrentRelativeHumidity": "homeBridge/airQualitySensor/CurrentRelativeHumidity"
            },
            "history": true,
            "room2": true
        }

[arachnetech]

Thanks, @dd4rk. Should that characteristic_TemperatureDisplayUnits( tempSvc ); be conditional on room2 being true too?

[D4rkCrypto]

For room2 setting, it's necessary to specify temperature display unit as Celsius. But we don't need to conditionally add this. It's alright with room1.

[arachnetech]

Thanks, so

                if( config.topics.getCurrentTemperature ) {
                    let tempSvc = new Service.TemperatureSensor( svcNames.temperature || name + "-Temperature", subtype );
                    characteristic_CurrentTemperature( tempSvc );
                    if( config.room2 ) {
                        characteristic_TemperatureDisplayUnits( tempSvc );
                    }
                    addSensorOptionalCharacteristics( tempSvc );
                    services.push( tempSvc );
                }

... and remove the line from the if( config.history && config.room2 ) { block

[arachnetech]

lol, ok I'll remove the if( config.room2 ) { then

[arachnetech]

pushed

[arachnetech]

Should be fixed in latest releases.