Espeasy: GPIOtoggle,x returns both json and plain text

Created on 17 Feb 2020  路  24Comments  路  Source: letscontrolit/ESPEasy

http://IP/control?cmd=GPIOtoggle,4

returns :

{
"log": "Toggle GPIO 4 Set to 1",
"plugin": 1,
"pin": 4,
"mode": "output",
"state": 1
}

Ok

Should be either JSON or OK but not both ?

Rules Bug

Most helpful comment

mega-20200204 is the release I noticed this behavior on
another unit on mega-20190805 behaves properly.

BTW this is not limited tot he GPIOtoglle as initially posted, as I just confirmed gpio,x,y pulse,x and longpulse,x

-D

All 24 comments

Should be either JSON or OK but not both ?

Yep.
What build is this?

Edit:
Just tested it and it also occurs with the latest build I have on my test node here.

@giig1967g Is this taken care of in your PR?

@ironataerial Can you test this test build

mega-20200204 is the release I noticed this behavior on
another unit on mega-20190805 behaves properly.

BTW this is not limited tot he GPIOtoglle as initially posted, as I just confirmed gpio,x,y pulse,x and longpulse,x

-D

@TD-er
the mega-20200204-33-PR_2778 returns only the text.
Used to return the JSON in the prior version.

-D

Thanks for testing/verifying.

@TD-er
The JSON response is preferred well over the text as it used to be.

The same (JSON and OK) remains in the mega-20200305
JSON only is what is preferred.

{
"log": "GPIO 4 Set to 1",
"plugin": 1,
"pin": 4,
"mode": "output",
"state": 1
}

Ok

Thank you
-D

That's correct as it has not yet been fixed (or a fix merged)

mega-20200204 is the release I noticed this behavior on
another unit on mega-20190805 behaves properly.

BTW this is not limited tot he GPIOtoglle as initially posted, as I just confirmed gpio,x,y pulse,x and longpulse,x

-D

thanks for mentioning a not affected release, after downgrading to mega-20190805 I can do parsing in node red without complications. :-)

OK, I've looked into the code and it seems some plugins do send the status themselves.
So this is a design flaw that was made apparent when I tried to remove duplicate code to make it more generic in handling the return values and status updates.

Some of these, the switch plugins and GPIO extenders, are now about to undergo a big change (see PR: https://github.com/letscontrolit/ESPEasy/pull/2778 ), but there are still others that will have the same issue:

  • P001 Switch
  • P009 MCP
  • P019 PCF8574
  • P011 PME
  • P022 PCA9685
  • P075 Nextion

The design flaw is that we don't properly detect if the status has been sent, but just assume it has by looking at the return value when calling the PLUGIN_WRITE and always return "\nOK"
The first 3 of these will be dealt with in the linked PR, but for the others we still have this issue.

Possible approaches to fix this:

  • Let the plugin set a (new) flag in the event when a status update was sent
  • Return the string to send as status update in the event.

this should be fixed in #2778, at least for P001, P009 and P019

this should be fixed in #2778, at least for P001, P009 and P019

It does not seem to have any affect on 20200328 tested just now,

http:///control?cmd=gpio,4,1

results to :

{
"log": "GPIO 4 Set to 1",
"plugin": 1,
"pin": 4,
"mode": "output",
"state": 1
}

Ok

It does not seem to have any affect on 20200328 tested just now,

That pull request has (still) not yet been merged.

It does not seem to have any affect on 20200410 tested just now,

Assume it has not been merged yet ?

It does not seem to have any affect on 20200410 tested just now,

Assume it has not been merged yet ?

That's correct.
It is included in the big GPIO PR and I didn't want to merge to many big changes at once.
So I will try to split that fix from that PR later this evening so it is no longer dependent.
After all it is a separate issue.

@TD-er
Hi, if I remember correctly, the change involves the new internal command structure.
If I may suggest, I think that you should merge the whole PR instead of just part of it. Also because my code is based on November release and its becoming difficult to maintain.
Maybe you could have a look at it and finally merge it.
Thanks

@giig1967g Problem with the current state of that PR is that it will break existing Domoticz (and maybe others too) integration and I don't have a clue yet why.

Hi,
Why don't you explain to me the problem?
With examples and cases.
I might help finding the cause.

Well I am not really sure your implementation is bad.
It may also be the way we did handle Domoticz stuff was the bad way after all.
It is truly unintuitive as it is done right now, but it is a fact it has been used a while so there are systems out there using exactly that way of switching. So it will be a breaking change.

Just about your remark of diverging from the main branch.
I did bring the pull request in sync with the main branch a few days ago, so you can make a branch from the current point of the pull request.
Not saying I should take much more time to look into it, but more like I don't want to complicate stuff for you.

It does not seem to have any affect on 20200426 tested just now.

It is also not been merged/fixed jet.

It does not seem to have any affect on 20200720 tested just now.

unfortunately no.
The PR has not been merged yet.

TD-er had a lot of work fixing wifi issues.

Hopefully in the next release.

The next (building right now) also does not have a fix for this...

Seems fixed on 20201022

Was this page helpful?
0 / 5 - 0 ratings

Related issues

wolverinevn picture wolverinevn  路  5Comments

thehijjt picture thehijjt  路  4Comments

jroux1 picture jroux1  路  6Comments

SANCLA picture SANCLA  路  4Comments

uzi18 picture uzi18  路  5Comments