BobRak / OpenHAB-Smartthings

53 stars 54 forks source link

Discover with payload > 45k from smartthings causing discover to fail. #27

Closed megaoldgeek closed 6 years ago

megaoldgeek commented 6 years ago

Greetings Bob.

I installed the new snapshot with my device handler set to all devices... clicked discover... waited and waited... then it timed out showing nothing to add.. So as any good solder I went to the logs :)

The only entry... 2018-01-08 23:12:08.075 [INFO ] [hings.internal.SmartthingsHttpClient] - Sent message "{"discovery": "yes"}" with path "/discovery" to the Smartthings hub, recieved HTTP status 202

Multiple times (because I clicked the discover more than once)... each time no other messages...

So, like an loyal soldier... I whipped up a simple script to debug what in the heck is going on here. server.txt

Fired up an ssh session on the openhab box and fired a at the hub... curl -H "Content-Type: application/json" -X POST -d '{"discovery":"true"}' http://192.168.1.123:39500/discover

Low and behold the smartthings hub is sending data... However after multiple posts it became clear the hub is truncating the body every time...

All my devices enabled = 45826 bytes in payload, seems to be too much. Output Excerpt from the attached script Expected:48352 In:826

Buffer LEN:42826 Buffer LEN:45826

Notice how the buffer length stops at 45826... and is repeated... Bottom line the code via the content-length header should receive 48352 bytes and only received 45826 so I never process the entire messages as the end of the JSON is not there..

To finish this and confirm what I found... I went into my hub and started unchecking devices.... to get my payload down. Seems the tip over point is anything > 45200. So removing all my switch indicators reduced, my payload is 40630 bytes. And multiple discover attempts seem to send the correct payload every time.

So it appears the handler code should limit its payload response / post < 45k, otherwise openhab is not going to get it all.

Hope this helps.

megaoldgeek commented 6 years ago

server.txt

BobRak commented 6 years ago

I reviewed the Smartthings code and the OpenHAB code and think there is a simple solution. It appears that the OpenHAB code could handle the response from the hub as multiple messages. I could change the Smartthings code to check the result size as it works through the devices. Anytime the result exceeds sa 40k I could send that back to openHAB and the continue building another result.

If I make a code change and post it here could you make the change to the Smartthings App and test since I don't have many devices.

Thanks,

Bob

megaoldgeek commented 6 years ago

I certainly can.

Cheers, -don

BobRak commented 6 years ago

Ok, I think I have a cure for this issue. If you would try it and let me know I would appreciate it. This is a change to the smartthings platform. Since you have already done the installation you must be familiar with the smartthings ide. You can refer to the SmarthingsInstallation document. Here are the instructions:

This will cause discovery to execute the revised function. If this works I'll make this change permanent and will roll it out with the fix to the CarbonMonoxideSensor.

Here is the new function:

`def openhabDiscoveryHandlerTest(evt) { log.debug "Entered test discovery handler" def results = [] def bufferLength = 0

CAPABILITY_MAP.each { key, capability ->
    capability["attributes"].each { attribute ->
        settings[key].each {device ->
            // The device info has to be returned as a string. It will be parsed into device data on the OpenHAB side
            def deviceInfo = "{\"capability\": \"${key}\", \"attribute\": \"${attribute}\", \"name\": \"${device.displayName}\", \"id\": \"${device.id}\" }" 
            results.push(deviceInfo)
            bufferLength += deviceInfo.length()
            // Check if we have close to a full buffer and if so send it
            if( bufferLength > 40000 ) {
                def json = new groovy.json.JsonOutput().toJson([
                    path: "/smartthings/discovery",
                    body: results
                ])
                log.debug "Discovery is returning JSON: ${json}"
                openhabDevice.deviceNotification(json)
                results = []
                bufferLength = 0
            }                
        }
    }
}

if( bufferLength > 0 ) {
    def json = new groovy.json.JsonOutput().toJson([
        path: "/smartthings/discovery",
        body: results
    ])
    log.debug "Discovery is returning FINAL JSON: ${json}"
    openhabDevice.deviceNotification(json)
}

}

`

Please let me know how it goes. I hope my instructions are clear enough.

Thanks for the testing and feedback. I'm going to change your name to megaDEVICESoldgeek.

Bob

megaoldgeek commented 6 years ago

Thanks Bob,

As a coder (not groovy literate) I got thru it :) The chunking code works great! Before I reloaded openhab I used my script... and found the > 40000 limit actually pushed ~44k ... obviously will vary with device name, id and such... so smarthings hub truncated as expected. So I changed it to 30000 and the chunking works perfectly! I haven't counted but it appears "ALL" my devices were discovered on openhab.

Nice job! -don

BobRak commented 6 years ago

OK, I'll change the max size to 30000 just to be safe. Tomorrow I'll add this to the git code base and will do a new build and will then push a new release to github. I'll send you a message when it is done so you know it is ready,

Bob

BobRak commented 6 years ago

Don (@megaoldgeek ):

OK, I have pushed the changes to the Smartthing hub SmartApp to github. No need for you to install this since it just has the code I sent you earlier.

Bob

megaoldgeek commented 6 years ago

This fix works great! Thanks!

-don