mdhiggins / ESP8266-HTTP-IR-Blaster

ESP8266 Compatible IR Blaster that accepts HTTP commands for use with services like Amazon Echo
MIT License
968 stars 217 forks source link

Adding /state endpoint to get current device state #317

Closed shuaiscott closed 1 year ago

shuaiscott commented 3 years ago

Just added /state and /send endpoints to check a device's state and to send with a json response so Homebridge plugins like homebridge-http-switch-webhook can use the data to show if the appliance is on or off.

mdhiggins commented 3 years ago

Nice. I've actually been using homebridge and the local bypass was so I could get that working with some security. How does your send endpoint differ from the existing JSON endpoint? I've been using that without any issues on homebridge. On mobile now so couldn't review the code changes extensively

shuaiscott commented 3 years ago

Yeah I gave myself a late night working on this. I’m brand new to Homebridge so I thought the plug-in most people were using required JSON. I later found a version that just did regex matching on the response to be able to detect the status. So probably a bit of a waste of time for myself haha. I’m not as familiar with your JSON endpoint, so I’ll look into that... but I couldn’t find any status endpoint for this... like one for the Homebridge plug-in to call to see what state it’s in without performing any action on the device. You can ignore the PR and I can refactor if you’d like the state check endpoint added to your main one. Either way 😅

On Sat, Apr 24, 2021 at 2:25 AM Michael Higgins @.***> wrote:

Nice. I've actually been using homebridge and the local bypass was so I could get that working with some security. How does your send endpoint differ from the existing JSON endpoint? I've been using that without any issues on homebridge. On mobile now so couldn't review the code changes extensively

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/mdhiggins/ESP8266-HTTP-IR-Blaster/pull/317#issuecomment-826057098, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABLDPO2BQ43KFIGXPMPABY3TKJ6BLANCNFSM43P2K6DQ .

mdhiggins commented 3 years ago

I mean there's really no reason a JSON response can't be included. The blueprint already has a JSON endpoint for sending a JSON object though so I was just wondering how your /json endpoint differed but it seems like its more about the response than the input. There is no /state equivalent so that would be good to include.

There is already a 'simple' response that can be send by including that in the URL parameters which sends a line of text instead of the full web interface that Alexa uses for its HTTP requests. I'm thinking the best option would be to include the JSON response if that mode is engaged for the /msg and /json endpoints instead and delete the /send endpoint but keep the /state endpoint

mdhiggins commented 3 years ago

https://github.com/emptygalaxy/homebridge-http-television

That's what I've been using for homebridge btw

shuaiscott commented 3 years ago

Awesome, thanks for the info! Yeah I just put in a commit to remove the /send endpoint. I think just using the normal response text should work for my needs. The /state endpoint is definitely one I'd like to keep if you're ok with the implementation. It makes it much easier to track the state.

And thanks again for building and hosting this! I put an ESP8266 in my electric fireplace and it works wonders being able to control it with IFTTT and Homebridge.

mdhiggins commented 3 years ago

Looks good, any particular reason why you're changing the default state from 0 to 1 in this pull request?

shuaiscott commented 3 years ago

Looks good, any particular reason why you're changing the default state from 0 to 1 in this pull request?

It makes sense to me that if the current state didn't exist in the map, that the default state there should be a 0. So if someone made a request without specifying the state, it wouldn't still be 0, it would be the next increment, or 1.

There is an argument to be made though if we're treating it as if devices can have a null state, in which case I guess that value should be 0.

There's also the case where me adding this value here might break things for people. So I can just remove that if that's desired. :)

Relevant piece of code with the default states and new default state:


int state = (server->hasArg("state")) ? server->arg("state").toInt() : 1;
           if (deviceState.containsKey(device)) {
             Serial.println("Contains the key!");
             Serial.println(state);
            int currentState = deviceState[device];
            Serial.println(currentState);
            if (state == currentState) {
              if (simple) {
                sendCorsHeaders();
                server->send(200, "text/html", "Not sending command to " + device + ", already in state " + state);
              } else {
                sendHomePage("Not sending command to " + device + ", already in state " + state, "Warning", 2); // 200
              }
              Serial.println("Not sending command to " + device + ", already in state " + state);
              return;
            } else {
              Serial.println("Setting device " + device + " to state " + state);
              deviceState[device] = state;```
mdhiggins commented 3 years ago

I'll include these changes in the upcoming MQTT update

mdhiggins commented 3 years ago

https://github.com/mdhiggins/ESP8266-HTTP-IR-Blaster/tree/mqtt-dev

Could use some testers for the new MQTT feature if you're interested