itead / Sonoff_Devices_DIY_Tools

BSD 3-Clause "New" or "Revised" License
548 stars 168 forks source link

Sonoff Mini zeroconf/info response #34

Open olyfunt opened 4 years ago

olyfunt commented 4 years ago

Here is what I receive (response body) after sending a request to zeroconf/info wanting to get the current status of the switch:

{"seq":168,"error":0,"data":"{\"switch\":\"off\",\"startup\":\"stay\",\"pulse\":\"off\",\"pulseWidth\":500,\"ssid\":\"XXXXX\",\"otaUnlock\":false}"}

Please note that the "data" attribute is actually a String value - not an object. Why is that? Shouldn't it be rather something like below? {"seq":168,"error":0,"data":{"switch":"off","startup":"stay","pulse":"off","pulseWidth":500,"ssid":"XXXXX","otaUnlock":false}}

I tested communication using ARC and curl.

ZZLinvec commented 4 years ago

Is the Sonoff Mini firmware version 3.3.0? @olyfunt

ZZLinvec commented 4 years ago

If so, its data field must be a json object.

olyfunt commented 4 years ago

The firmware version is 3.3.0. I upgraded the device right after I received it last week and before I switched to DIY mode and mounted in the wall.

olyfunt commented 4 years ago

I didn't notice it earlier but even DIY tool shows that data is a String value enclosed in apostrophes. The device shown in the image is one I unpacked and updated a few moments ago and the firmware is of course 3.3.0, the latest available.

Sonoff_mini_info

ZZLinvec commented 4 years ago

@olyfunt This section displays the json data to a string, just to show the return information from getting the device data.

olyfunt commented 4 years ago

Why Closed? No answer has been given yet. The question was not WHETHER "data" field is an object but WHY IT IS NOT an object.

Here is how /zeroconf/info response looks like when requested using curl: curl

Please copy the raw HTTP response body I provided initially:

{"seq":168,"error":0,"data":"{\"switch\":\"off\",\"startup\":\"stay\",\"pulse\":\"off\",\"pulseWidth\":500,\"ssid\":\"XXXXX\",\"otaUnlock\":false}"}

paste it in any online JSON parser and check for yourself. json_parse

When a piece of software receives such a response and parse it, it gets an object with 3 fields like below: { "seq": number, "error": number, "data": string } Despite the things written here: https://github.com/itead/Sonoff_Devices_DIY_Tools/blob/master/SONOFF%20DIY%20MODE%20Protocol%20Doc%20v1.4.md#8-get-device-info forget about reading e.g. data.switch to check whether it is "on" or "off". This is because "data" field is just a plain text only containing JSON definition. It needs to be parsed yet in order to get an object and read the fields inside.

I can deal with it if I have to. I was only curious if this a bit strange solution serves any purpose. Do you see my point?

ZZLinvec commented 4 years ago

sorry,This is my fault. I have rechecked the information returned by the device. At present, this information is indeed of type string.I always thought that you asked what type is in the data field, so I am really sorry for the inconvenience caused to you.

ZZLinvec commented 4 years ago

This is a mistake in the firmware, we will improve it.

olyfunt commented 4 years ago

Thank you @ZZLinvec This is a Sonoff device I was waiting for, that's why I care.

JAMESBOWLER commented 4 years ago

I found the same issue I have it running in openhab. I just transform the data object into anthrer string being a json one and then use that as json data to put all the data into relevant item.

` if (UpdateResult.contains('"error":0')) { JSONdata = transform("JSONPATH", "$.data", UpdateResult)
// logInfo("Sonoff Poll JSONdata", JSONdata) ``

  if (JSONdata != Last_update) {

    update_switch_state = transform("JSONPATH", "$.switch", JSONdata)

`

alexbk66 commented 4 years ago

Yeah, works for me: SonoffData data = JsonConvert.DeserializeObject<SonoffData>(data);

public class SonoffData
{
    public onoff @switch { get; set; }
    public onoff startup { get; set; }
    public onoff pulse { get; set; }
    public int pulseWidth { get; set; }
    public int rssi { get; set; }
}
alexbk66 commented 4 years ago

@ZZLinvec - if you now change the firmware - you will break existing code. Is it worths it? And I wonder - do you follow proper software development procedures, do you do testing?

frenck commented 4 years ago

From the consumer endpoint, a change for this can be mitigated pretty easily by checking the type of the data payload.

IMHO, this should be fixed ASAP. The longer you guys wait on fixing this, the harder it becomes.

alexbk66 commented 4 years ago

From the consumer endpoint, a change for this can be mitigated pretty easily by checking the type of the data payload.

Yeah, "pretty easy"? But existing software will have to be updated before the firmware update.

Daniel-zhan-itead commented 4 years ago

We will update zeroconf/info response body as object but not strings, in the next firmware.

frenck commented 4 years ago

Thanks, @Daniel-zhan-itead

alexbk66 commented 4 years ago

We will update zeroconf/info response body as object but not strings, in the next firmware.

Hi Daniel @Daniel-zhan-itead, just wondering - are you planning to implement OTA FW update for DIY? Or we have to go off DIY mode to be able to use the eWeLink app to update FW and then go through the complicate pairing procedure to go back to DIY mode?