Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HandleHttpCommand() to return application/json content-type header #1363

Closed
BrettSheleski opened this issue Dec 14, 2017 · 6 comments
Closed
Labels
stale Action - Issue left behind - Used by the BOT to call for attention

Comments

@BrettSheleski
Copy link

Currently HandleHttpCommand() return the content-type of text/plain with content that is JSON-ish.

Proposal:

Check the response's Accept header. If it is set to "application/json" then set the response to "application/json". The content will have to be modified in this case to be valid json. It seems that all responses follow the format of "RESULT = " followed by json. There are cases (eg: /cm?cmnd=gpios) which returns several lines in the format of "RESULT = " followed by json on each line. Therefore it may make sense to wrap all responses in a json array.

Something like (over simplified code):

String json = "[";

for(int i = 0; i < numberOfMessages; ++i){
  if (i > 0) json += ",";

  json += getCommandResultJson(i);
}

json += "]"

...

WebServer->send(200, FPSTR(HDR_CTYPE_JSON), json);

...

Why bother?

I'm currently implementing a new set of SmartThings Device Handlers & SmartApps. There is a limitation (aka: bug) within SmartThings where if the response's content type is anything other than application/json or text/xml, then it is not accessible by its callback methods handling the response. The above seems like a fairly simple solution to this problem. I could also see it being useful for other use cases as well.

@arendst
Copy link
Owner

arendst commented Dec 17, 2017

Looked at it and think I found a reasonable alternative.

  1. Remove result (or any value as this can be changed with command SetOption4)
  2. Return only valid JSON by adding JSON lines removing {} if needed:
{"GPIOs1":"0 (None),1 (DHT11),2 (AM2301),3 (SI7021),4 (DS18x20),5 (I2C SCL),6 (I2C SDA),7 (WS2812),8 (IRsend),9 (Switch1),10 (Switch2),11 (Switch3),12 (Switch4),13 (Button1),14 (Button2),15 (Button3),16 (Button4)","GPIOs2":"17 (Relay1),18 (Relay2),19 (Relay3),20 (Relay4),21 (Relay5),22 (Relay6),23 (Relay7),24 (Relay8),25 (Relay1i),26 (Relay2i),27 (Relay3i),28 (Relay4i),29 (Relay5i),30 (Relay6i),31 (Relay7i),32 (Relay8i)","GPIOs3":"33 (PWM1),34 (PWM2),35 (PWM3),36 (PWM4),37 (PWM5),38 (Counter1),39 (Counter2),40 (Counter3),41 (Counter4),42 (PWM1i),43 (PWM2i),44 (PWM3i),45 (PWM4i),46 (PWM5i),47 (IRrecv),48 (Led1),49 (Led2)","GPIOs4":"50 (Led3),51 (Led4),52 (Led1i),53 (Led2i),54 (Led3i),55 (Led4i),56 (MHZ Tx),57 (MHZ Rx),58 (PZEM Tx),59 (PZEM Rx)"}

Using lint:

{
	"GPIOs1": "0 (None),1 (DHT11),2 (AM2301),3 (SI7021),4 (DS18x20),5 (I2C SCL),6 (I2C SDA),7 (WS2812),8 (IRsend),9 (Switch1),10 (Switch2),11 (Switch3),12 (Switch4),13 (Button1),14 (Button2),15 (Button3),16 (Button4)",
	"GPIOs2": "17 (Relay1),18 (Relay2),19 (Relay3),20 (Relay4),21 (Relay5),22 (Relay6),23 (Relay7),24 (Relay8),25 (Relay1i),26 (Relay2i),27 (Relay3i),28 (Relay4i),29 (Relay5i),30 (Relay6i),31 (Relay7i),32 (Relay8i)",
	"GPIOs3": "33 (PWM1),34 (PWM2),35 (PWM3),36 (PWM4),37 (PWM5),38 (Counter1),39 (Counter2),40 (Counter3),41 (Counter4),42 (PWM1i),43 (PWM2i),44 (PWM3i),45 (PWM4i),46 (PWM5i),47 (IRrecv),48 (Led1),49 (Led2)",
	"GPIOs4": "50 (Led3),51 (Led4),52 (Led1i),53 (Led2i),54 (Led3i),55 (Led4i),56 (MHZ Tx),57 (MHZ Rx),58 (PZEM Tx),59 (PZEM Rx)"
}

Or status 10:

{"StatusSNS":{"Time":"2017-12-17T15:46:58","ENERGY":{"Total":0.000,"Yesterday":4.000,"Today":0.000,"Power":0.00,"Factor":0.00,"Voltage":0.00,"Current":0.000},"SHT1X":{"Temperature":23.6,"Humidity":46.3},"HTU21":{"Temperature":24.4,"Humidity":44.7},"BMP280":{"Temperature":25.7,"Pressure":1022.7,"SeaPressure":1023.2},"TempUnit":"C"}}

using lint:

{
	"StatusSNS": {
		"Time": "2017-12-17T15:46:58",
		"ENERGY": {
			"Total": 0.000,
			"Yesterday": 4.000,
			"Today": 0.000,
			"Power": 0.00,
			"Factor": 0.00,
			"Voltage": 0.00,
			"Current": 0.000
		},
		"SHT1X": {
			"Temperature": 23.6,
			"Humidity": 46.3
		},
		"HTU21": {
			"Temperature": 24.4,
			"Humidity": 44.7
		},
		"BMP280": {
			"Temperature": 25.7,
			"Pressure": 1022.7,
			"SeaPressure": 1023.2
		},
		"TempUnit": "C"
	}
}

Even the error messages are now JSON:

{"WARNING":"Enable weblog 2 if response expected"}

Soon provided by next Commit to 5.10.0b

arendst added a commit that referenced this issue Dec 17, 2017
* Change Wemo SetBinaryState to distinguish from GetBinaryState (#1357)

* Change output of HTTP command to valid JSON only (#1363)
@BrettSheleski
Copy link
Author

Nice! Thanks.

@BrettSheleski
Copy link
Author

Taking a look at your example output for "gpios", wouldn't it make more sense to return each result as an array?

Instead of the following

{
	"GPIOs1": "0 (None),1 (DHT11),2 (AM2301),3 (SI7021),4 (DS18x20),5 (I2C SCL),6 (I2C SDA),7 (WS2812),8 (IRsend),9 (Switch1),10 (Switch2),11 (Switch3),12 (Switch4),13 (Button1),14 (Button2),15 (Button3),16 (Button4)",
	"GPIOs2": "17 (Relay1),18 (Relay2),19 (Relay3),20 (Relay4),21 (Relay5),22 (Relay6),23 (Relay7),24 (Relay8),25 (Relay1i),26 (Relay2i),27 (Relay3i),28 (Relay4i),29 (Relay5i),30 (Relay6i),31 (Relay7i),32 (Relay8i)",
	"GPIOs3": "33 (PWM1),34 (PWM2),35 (PWM3),36 (PWM4),37 (PWM5),38 (Counter1),39 (Counter2),40 (Counter3),41 (Counter4),42 (PWM1i),43 (PWM2i),44 (PWM3i),45 (PWM4i),46 (PWM5i),47 (IRrecv),48 (Led1),49 (Led2)",
	"GPIOs4": "50 (Led3),51 (Led4),52 (Led1i),53 (Led2i),54 (Led3i),55 (Led4i),56 (MHZ Tx),57 (MHZ Rx),58 (PZEM Tx),59 (PZEM Rx)"
}

return the following instead:

{
	"GPIOs1": ["0 (None)","1 (DHT11)","2 (AM2301)","3 (SI7021)","4 (DS18x20)", ...],
	"GPIOs2": ["17 (Relay1)", ...],
	"GPIOs3": [ ... ],
	...
}

Therefore no further parsing would be necessary by any clients other than json-parsing.

On a side note, why are these returned as multiple properties anyway? Wouldn't it make more sense to just return as a single large array?

@BrettSheleski BrettSheleski reopened this Dec 18, 2017
@arendst
Copy link
Owner

arendst commented Dec 18, 2017

I agree on the Array usage and will implement.

It's multiple properties because of the maximum allowed MQTT string size. It overflows the (current) maximum string length leading to missed data and non JSON format. A larger string will influence RAM and performance of the ESP8266.

@BrettSheleski
Copy link
Author

Gotcha. I figured it had something to do with that.

arendst added a commit that referenced this issue Dec 18, 2017
* Change output to valid JSON Array if needed (#1363)
arendst added a commit that referenced this issue Jan 7, 2018
5.11.0 20180107
 * Minor webpage HTML optimizations (#1358)
 * Updated
German translation (#1438)
 * Change Sonoff Pow Energy MQTT data message
and consolidate Status 8 into Status 10
 * Change ADS1115 default
voltage range from +/-2V to +/-6V (#1289)
 * Change text to Active for 3
minutes (#1364)
 * Change Wemo SetBinaryState to distinguish from
GetBinaryState (#1357)
 * Change output of HTTP command to valid JSON
and Array only (#1363)
 * Removed all MQTT, JSON and Command language
defines from locale files and set fixed to English (#1473)
 * Renamed
commands Color2,3,4 to Color3,4,5
 * Fix BME280 calculation (#1051)
 *
Fix Sonoff Bridge missed learned key if learned data contains 0x55 (End
of Transmission) flag (#1095, #1294)
 * Fix PWM initialization in
Dimmer/Color mode (#1321)
 * Fix Wemo Emulation (#1357)
 * Fix display
of build date and time in non-english locale (#1465)
 * Fix Wemo and Hue
emulation by adding M-Search response delay (#1486)
 * Add libraries
Adafruit_BME680-1.0.5, Adafruit_Sensor-1.0.2.02, TasmotaSerial-1.0.0 and
TSL2561-Arduino-Library
 * Add command Color2 to set color while keeping
same dimmer value
 * Add device function pointers
 * Add support for
SenseAir S8 CO2 sensor
 * Add color led signal to Carbon Dioxide (CO2)
sensors using defines CO2_LOW and CO2_HIGH in user_config.h
 * Add
support for Domoticz Air Quality sensor to be used by MH-Z19(B) and
SenseAir sensors
 * Add support for PZEM004T energy sensor
 * Add
support for iTead SI7021 temperature and humidity sensor by
consolidating DHT22 into AM2301 and using former DHT22 as SI7021 (#735)

* Add support for BME680 using adafruit libraries (#1212)
 * Add support
for MH-Z19(B) CO2 sensor (#561, #1248)
 * Add multipress support and
more user configurable GPIO to Sonoff Dual R2 (#1291)
 * Add support for
TSL2561 using adafruit library (#661, #1311)
 * Add support for SHT3x
(#1314)
 * Add support for Arilux LC06 (#1414)
 * Add Italian language
file (#1449)
 * Add 2nd Gen Alexa support to Wemo emulation discovery
(#1357, #1450)
 * Add define for additional number of WS2812 schemes
(#1463)
joecotton pushed a commit to joecotton/Sonoff-Tasmota that referenced this issue Jan 8, 2018
5.11.0 20180107
 * Minor webpage HTML optimizations (arendst#1358)
 * Updated
German translation (arendst#1438)
 * Change Sonoff Pow Energy MQTT data message
and consolidate Status 8 into Status 10
 * Change ADS1115 default
voltage range from +/-2V to +/-6V (arendst#1289)
 * Change text to Active for 3
minutes (arendst#1364)
 * Change Wemo SetBinaryState to distinguish from
GetBinaryState (arendst#1357)
 * Change output of HTTP command to valid JSON
and Array only (arendst#1363)
 * Removed all MQTT, JSON and Command language
defines from locale files and set fixed to English (arendst#1473)
 * Renamed
commands Color2,3,4 to Color3,4,5
 * Fix BME280 calculation (arendst#1051)
 *
Fix Sonoff Bridge missed learned key if learned data contains 0x55 (End
of Transmission) flag (arendst#1095, arendst#1294)
 * Fix PWM initialization in
Dimmer/Color mode (arendst#1321)
 * Fix Wemo Emulation (arendst#1357)
 * Fix display
of build date and time in non-english locale (arendst#1465)
 * Fix Wemo and Hue
emulation by adding M-Search response delay (arendst#1486)
 * Add libraries
Adafruit_BME680-1.0.5, Adafruit_Sensor-1.0.2.02, TasmotaSerial-1.0.0 and
TSL2561-Arduino-Library
 * Add command Color2 to set color while keeping
same dimmer value
 * Add device function pointers
 * Add support for
SenseAir S8 CO2 sensor
 * Add color led signal to Carbon Dioxide (CO2)
sensors using defines CO2_LOW and CO2_HIGH in user_config.h
 * Add
support for Domoticz Air Quality sensor to be used by MH-Z19(B) and
SenseAir sensors
 * Add support for PZEM004T energy sensor
 * Add
support for iTead SI7021 temperature and humidity sensor by
consolidating DHT22 into AM2301 and using former DHT22 as SI7021 (arendst#735)

* Add support for BME680 using adafruit libraries (arendst#1212)
 * Add support
for MH-Z19(B) CO2 sensor (arendst#561, arendst#1248)
 * Add multipress support and
more user configurable GPIO to Sonoff Dual R2 (arendst#1291)
 * Add support for
TSL2561 using adafruit library (arendst#661, arendst#1311)
 * Add support for SHT3x
(arendst#1314)
 * Add support for Arilux LC06 (arendst#1414)
 * Add Italian language
file (arendst#1449)
 * Add 2nd Gen Alexa support to Wemo emulation discovery
(arendst#1357, arendst#1450)
 * Add define for additional number of WS2812 schemes
(arendst#1463)
@stale
Copy link

stale bot commented Apr 23, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Action - Issue left behind - Used by the BOT to call for attention label Apr 23, 2018
curzon01 pushed a commit to curzon01/Tasmota that referenced this issue Sep 6, 2018
* Change Wemo SetBinaryState to distinguish from GetBinaryState (arendst#1357)

* Change output of HTTP command to valid JSON only (arendst#1363)
curzon01 pushed a commit to curzon01/Tasmota that referenced this issue Sep 6, 2018
* Change output to valid JSON Array if needed (arendst#1363)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Action - Issue left behind - Used by the BOT to call for attention
Projects
None yet
Development

No branches or pull requests

2 participants