Add Transcriber and Portal objects. #215
Conversation
| #define ANALOG_IN A0 | ||
| #define ANALOG_OUT D8 | ||
|
|
||
| thing::Config config = { |
There was a problem hiding this comment.
I wonder if we should keep the namespace in the Arduino surface, iirc, it's not very common.
There was a problem hiding this comment.
Well I think this has to do with splitting the library, This example isn't really meant to be arduino developer friendly (like the config struct isn't too friendly), the example is just to give people a chance to play with the Transcriber outside of the rest of the system.
| } | ||
|
|
||
| void Transcriber::SetValue(const std::string& path, const std::string& value) { | ||
| stream_.reset(nullptr); |
There was a problem hiding this comment.
does that close the connection?
There was a problem hiding this comment.
Yes, I added http->end() to the FirebaseCall destructor, I don't think it is necessary as the Esp8266HttpClient destructor is doing this too but since we have a portability layer everything in the future may not.
| // Send values to cloud | ||
| int digital_in = digitalRead(pin_digital_in_); | ||
| if (digital_in != digital_in_) { | ||
| SetValue(path_ + kSubPathDigitalIn, String(digital_in).c_str()); |
There was a problem hiding this comment.
I wonder if we could batch those?
There was a problem hiding this comment.
We definitely could. It would be a little work to build the json stub to send, sending the raw values is far easier.
There was a problem hiding this comment.
I think it could be easily 2x faster, as the tax for each http request is quite big.
There was a problem hiding this comment.
can you create issue for those?
There was a problem hiding this comment.
Added a new issue #222
| } | ||
|
|
||
| float analog_in = analogRead(pin_analog_in_); | ||
| if (analog_in != analog_in_) { |
There was a problem hiding this comment.
we should probably have a threshold
There was a problem hiding this comment.
I have been considering that as well but I am unsure if we can define a common threshold. I haven't done enough analog reading. I usually define one empirically.
There was a problem hiding this comment.
the threadshold could be part of the config?
There was a problem hiding this comment.
I am not against it, lets consider this a future feature.
There was a problem hiding this comment.
The reason I'm worried here is that no threshold might trigger a send on each loop.
There was a problem hiding this comment.
can you create issue for those?
There was a problem hiding this comment.
Went ahead and implemented this as it wasn't too much work.
|
Added the Portal.{h,cc} changes in here as well. Seemed easier since dependent PRs are hard to parse (too much noise). |
|
oh nooo :) |
| @@ -0,0 +1,112 @@ | |||
| #include "thing/Portal.h" | |||
There was a problem hiding this comment.
should we use https://github.com/tzapu/WiFiManager instead or rolling something on our own?
There was a problem hiding this comment.
hmm maybe we could evaluate it at least.
There was a problem hiding this comment.
Unless there are issues with what is presented here, let's get this checked in and I will evaluate the WiFiManager this friday and let you know what I think.
|
Ya smaller PRs are better but I haven't figured out a good workflow for depending PRs yet that aren't a mess to review and merge. |
having them in two different (but dependent) branch and using rebase worked well for me in the past. But LGTM :) |
This will be the core of the new firethings project. It is responsible for connecting pins to the cloud and the cloud to pins.