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

blynk support #1230

Open
wants to merge 36 commits into
base: dev
Choose a base branch
from
Open

blynk support #1230

wants to merge 36 commits into from

Conversation

thaeger71
Copy link

please check for commit to dev branch.
Fyi, I disabled Thingspeak because of the firmware size, so that's still possible to make OTA update.
Maybe it would better to disable Alexa by default.

@mcspr
Copy link
Collaborator

mcspr commented Sep 26, 2018

Couple of notes:

  • how exactly BlynkSimpleStream.h gets installed? can pio download it?
  • no need to add gz, gz.h, they will be rebuilt outside pr
  • looks like package-lock.json and pio.ini device got added by accident?
  • please do not touch default settings. keep it disabled by default. just recommend turning off other similar providers (thingspeak etc.)
  • what happens when BLYNK_HOST is unavailable?

@thaeger71
Copy link
Author

BlynkStream.h is part of the Blynk client library. Blynk client library can be downloaded by pio, current version is 0.5.4

I didn't add any files except blynk.ino so far I know. Should I remove all the gz files, package-lock.json ? But I changed platformio.ini because of the new device ....so that can be built with pio.

Will reset defaults.

In the loop will be tried to connect to blynk server with a timeout of 3 seconds. On timeout the connect function returns to loop. This means every unsuccessful try will block loop for 3 seconds, I tried this and it didn't block the device in a bad way.

@thaeger71
Copy link
Author

I checked the original repository, all gz and lock and so on are already included .

@mcspr
Copy link
Collaborator

mcspr commented Sep 30, 2018

Yes, they are included, but as you can see — merging is not possible :) They will be generated anyways, no need to add them here.

Also, would it make sense to recommend disabling mqtt / http-api / webui html when on blynk? It pretty much replaces all of them, besides actual configuration.

@thaeger71
Copy link
Author

ok, what exactly do you want that I do ? Should I go back with all gz files and leave the other ones ? Why are html and platformio.ini not mergable ?

regarding disabling mqtt/http-api/webui, I can't decide, maybe there are people using techniques in parallel. Also, mybe I'am wrong, but is it possible to use espurna without webui ?

@mcspr
Copy link
Collaborator

mcspr commented Oct 3, 2018

It's not that I want it, it is what git wants. Basically, the text patch cannot apply.

It uses nearby lines and since they are not exact match between branches - it bails out and requires manual edit. Either here on github or using some local git tool - try to rebase on current 'dev'. It'll show what files are affected and format difference inline, like this (index.html):

<<<<<<< HEAD
                </form> <--- patch cannot apply because this line was not expected
=======
                .... <---- this is what you added by your commit, conflicting with previous line
>>>>>>> 8676877e...

Fortunately, required changes are clear enough: remove git markers, keep </form> and add <form id="form-blynk" class="pure-form form-settings">...</form> for blynk panel.

Same with ini file. And as you can imagine, comparing binary files line-by-line is problematic. Thus it is better to keep them outside of prs

code/espurna/config/hardware.h Outdated Show resolved Hide resolved
code/espurna/config/general.h Outdated Show resolved Hide resolved
@mcspr
Copy link
Collaborator

mcspr commented Oct 9, 2018

Also, see: https://travis-ci.org/xoseperez/espurna/jobs/436669250#L534
It does not show up in github checks panel here for some reason.

I did try this out and it got some problems:

  • if blynk server is unavailable / key is invalid / server does not respond with what client expects - serial loop is locked up periodically. And it does not seem the timeout blynk-client uses is increasing, so it is happening every 3 sec (via Blynk.connect(3000))
  • setting vpin to 0 and configuring button on the dashboard for it throws exception 28 when longpressing.
  • how exactly relay value is shown? debug output mentioned configuring relay but i cannot find relevant element in the dashboard

Capturing used protocol and doing some searching on how it is implemented, maybe port to AsyncTCP like thingspeak/influx support should be done instead?
https://github.com/blynkkk/blynkkk.github.io/blob/master/BlynkProtocol.md
https://github.com/blynkkk/blynk-library/blob/master/tests/pseudo-library.py

@thaeger71
Copy link
Author

thaeger71 commented Oct 10, 2018 via email

@mcspr
Copy link
Collaborator

mcspr commented Oct 18, 2018

Yep, phone app. I'll cite the blynk.cc :)

It's a digital dashboard where you can build a graphic interface for your project by simply dragging and dropping widgets.

I only now got around to testing this again and VPin worked fine as switch, real gpio as pushbutton. No reset this time, probably because i did not assign vpin manually (or some other user error)

About the timeout... It would be nice to have exponential backoff when connection is unsuccessful. Blynk lib does not do it itself, so I guess it can be done manually checking .connect() result and waiting a bit before calling it again (increasing time each failed attempt but not indefinitely). Even if you are doing async approach. (and btw, besides blynk.connect timeout, there is also 5sec timeout on WifiClient itself linked to both DNS and connection.)

Regardless, it does not break things horribly but makes serial/telnet interaction real slow and leds blinking is disrupted too. Networking itself still works, Wificlient makes sure of that by yielding once in a while.

@thaeger71
Copy link
Author

is there something I have to do more than requested changes ?

@mcspr
Copy link
Collaborator

mcspr commented Nov 18, 2018

Sorry for delay. I am fixing some things with this and will share it with you to review.

@mcspr
Copy link
Collaborator

mcspr commented Nov 20, 2018

@thaeger71 Thx! Note about 128 pins limit origin - https://community.blynk.cc/t/using-127-virtual-pins-in-sketch/30491/11. For general use they have bulk of special functions per pin like BlynkWidgetRead64, BlynkWidgetWrite120 used from definitions like BLYNK_WRITE(V64) etc. and there are only 128 of them.

As a finishing touch I wanted to add SecureClient version (testing it right now), with pinned self-signed cert support. blynk-cloud only supports self-signed as openssl s_client -host blynk-cloud.com -port 8441 shows, but local install can also use classic cert chained to CA (which I had not checked yet on how this would live together)
And I also noticed Terminal widget, but it can wait.

@mcspr
Copy link
Collaborator

mcspr commented Nov 21, 2018

@xoseperez this on par with domoticz / thingspeak - sync relays, send sensor data, - but can also be modified to support any input / output as vpins can hold anything (up to 128 bytes in size)

api for web is slightly different from thingspeak though, as it uses empty strings instead of zeroes. thus added methods to check settings value length

secure client variant requires heavy modifications to default config because in my case i see +123kb increase in flash size. things like web/mqtt/tngspk etc. should be disabled for it to be reasonable in size.

edit: and if this is mergeable i'd really suggest squashing to avoid adding 30 commits 🌵

// const char _blynk_cert[] PROGMEM = R"CERT(
// ...
// )CERT";
#ifndef BLYNK_USE_CUSTOM_CERT
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did add this mixing two different names and did not really think about placement. Given some other modules can also have overridable certs, maybe do either
config/override/blynk_certitificate.h <-> with default config/blynk_certificate.h
or
static/override/blynk_certificate.h <-> with default static/blynk_certificate.h

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, BLYNK_USE_CUSTOM_CERT -> BLYNK_SECURE_CERT_OVERRIDE?

// use default settings from Blynk itself to set proper port
if (host.equals(F(BLYNK_DEFAULT_DOMAIN))) {
if (BLYNK_SECURE_CLIENT) {
port = BLYNK_DEFAULT_PORT_SSL;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future they plan to support 443 instead of 8441, so better to override it here based on the library.

@mcspr mcspr mentioned this pull request Feb 21, 2019
@mcspr mcspr added this to the 1.14.0 milestone Feb 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants