nugget / python-insteonplm

Python 3 asyncio module for interfacing with Insteon Powerline modems
MIT License
33 stars 19 forks source link

Add battery level support for Hidden Door Sensor (2845-222) #2

Open nugget opened 7 years ago

nugget commented 7 years ago

(Relocated from Issue #1) from @rstanley75

Did a little investigation into the OpenHAB binding and I discovered something.

From src/main/resources/device_features.xml:

<feature name="HiddenDoorSensorData">
    <message-dispatcher>SimpleDispatcher</message-dispatcher>
    <message-handler cmd="0x03" group="1">NoOpMsgHandler</message-handler>
    <message-handler cmd="0x11" group="1">NoOpMsgHandler</message-handler>
    <message-handler cmd="0x13" group="1">NoOpMsgHandler</message-handler>
    <message-handler cmd="0x2e">HiddenDoorSensorDataReplyHandler</message-handler>
    <command-handler command="OnOffType">NoOpCommandHandler</command-handler>
    <poll-handler>NoPollHandler</poll-handler>
</feature>

Then, from src/main/java/org/openhab/binding/insteonplm/internal/device/MessageHandler.java:

    public static class HiddenDoorSensorDataReplyHandler extends MessageHandler {
        HiddenDoorSensorDataReplyHandler(DeviceFeature p) {
            super(p);
        }

        @Override
        public void handleMessage(int group, byte cmd1, Msg msg, DeviceFeature f, String fromPort) {
            InsteonDevice dev = f.getDevice();
            if (!msg.isExtended()) {
                logger.trace("{} device {} ignoring non-extended msg {}", nm(), dev.getAddress(), msg);
                return;
            }
            try {
                int cmd2 = msg.getByte("command2") & 0xff;
                switch (cmd2) {
                    case 0x00: // this is a product data response message
                        int batteryLevel = msg.getByte("userData4") & 0xff;
                        int batteryWatermark = msg.getByte("userData7") & 0xff;
                        logger.debug("{}: {} got light level: {}, battery level: {}", nm(), dev.getAddress(),
                                batteryWatermark, batteryLevel);
                        m_feature.publish(new DecimalType(batteryWatermark), StateChangeType.CHANGED, "field",
                                "battery_watermark_level");
                        m_feature.publish(new DecimalType(batteryLevel), StateChangeType.CHANGED, "field",
                                "battery_level");
                        break;
                    default:
                        logger.warn("unknown cmd2 = {} in info reply message {}", cmd2, msg);
                        break;
                }
            } catch (FieldException e) {
                logger.error("error parsing {}: ", msg, e);
            }
        }
    }

It looks like it's sending cmd 2e and then getting the battery level from the User Data 4 byte of the extended-length response.

On page 9 from the dev docs: hiddendoorpage9

I had originally interpreted that little bit on page 9 as returning the level at which the low battery level alert is tripped, but since I get convincingly accurate battery level info from OpenHAB perhaps Data 4 is the actual battery level and Data 7 is actually the level where the alert gets tripped?

Edit: Finally read the docs on Markdown formatting.

teharris1 commented 6 years ago

@nugget Looking at this request has me thinking about my design descision to break complex devices (like FanLinc) into different devices at the Insteon level. I think the best way to accomplish this type of monitoring is to introduce a sensor to any battery operated device and allow that to publish it's status. However this would (if I put a HA hat on) look like a separate device. If we did that then everything would be a separate device. I don't think that is a good idea.

Here is my thought. In insteonplm we have multiple states (ie. sensorState, batteryLevel, fanSpeed) But all in one device. In HA we break any state we want to manage/monitor into a separate device. This way the battery level could be a sensor (or binary_sensor) in HA but not require complex coding in insteonplm. Thoughts?

creack commented 5 years ago

Expose it as a sensor would work. Or it could be like the remotes and be an event upon low battery? (Not sure how Insteon reports the level)