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

GPIO14 changes from '0' to '1' where it shouldn't.. #1265

Closed
Oxyandy opened this issue Apr 17, 2018 · 30 comments
Closed

GPIO14 changes from '0' to '1' where it shouldn't.. #1265

Oxyandy opened this issue Apr 17, 2018 · 30 comments
Labels
Category: Plugin Related to supported sensors Type: Bug Considered a bug
Milestone

Comments

@Oxyandy
Copy link

Oxyandy commented Apr 17, 2018

Summary of the problem

GPIO14 most definitely changes from '0' to '1' where it should not..
GPIO14 shows wrongly as '1' on device page & reports to Domoticz the same '1' as the ESP
This is a working log from Domoticz of the float SW I have across GPIO14

13:17:56 | On <- Float SW rises changing from 'closed circuit' to 'open circuit'
13:17:56 | On
(takes Float SW 2 or 3 seconds to lower)
13:17:58 | Off <- Float SW lowers changing from 'open circuit' to 'closed circuit'
13:17:58 | Off

Same log on >Dev_11 Firmware (or greater), but has a problem.

11:17:14 | On <- Float SW rises changing from 'closed circuit' to 'open circuit'
11:17:14 | On
(takes float 2 or 3 seconds to lower)-Change in the switch still happens at the right time BUT
11:17:17 | On <- Wrong, should be OFF
11:17:18 | On

Expected behaviour

Have the change in the physical switch match what is represented on the Device page

Actual behavior

The wrong state is shown on the Device page & wrong state is reported to Domoticz

Steps to reproduce

Dev_11 Firmware default settings
Enable a Switch - I use GPIO14, internal pulldown & Domoticz MQTT
I then using the Devices web-page & a jumper cable from GPIO14 to GND
Simulate On-Off-On, and then replace jumper as a 'Closed Circuit'
A few F5 presses while doing above, and it will soon show wrongly as '1'
otherwise repeat as above a few more times.

Does the problem persists after powering off and on?

Yes

System configuration

In my case a Sonoff Basic with Dev_11 or greater firmware
Dev_4, 5, 6, 7, 8, 9 & 10 are confirmed to not have this issue.

So I tried all other controllers & could not replicate the issue
100% Domoticz MQQT code related.
_C002.ino did not change between Dev_10 & Dev_11
_CPlugin_DomoticzHelper.ino did not exist
_P029_Output.ino changed in Dev_11

I created an archive to help
Inside are only the changed files from Dev_10 to Dev_11, all renamed in the format below to make the origin of each file perfectly clear - Extract contents of this archive to a single folder

_P029_Output_Dev_10.ino
_P029_Output_Dev_11.ino

Selecting both versions, compare with a visual text compare tool.
If someone has the time ? Maybe something stands out ?
Dev_10 works, Dev_11 the problem started.

CHANGED_v2.0.0-dev11.zip
Maybe it is time I tried a more recent release of the firmware too,
see if any other changes have impacted anything..

The last firmware of the more recent ones I have tried was
ESP_Easy_mega-20180322_normal_ESP8266_1024.bin & it had this issue.
then I started back tracking to find a point the bug vanished.
I have not yet tried anything released after this version.

@Grovkillen Grovkillen added Type: Bug Considered a bug Category: Plugin Related to supported sensors labels Apr 17, 2018
@Grovkillen Grovkillen added this to the 2.0.0 milestone Apr 17, 2018
@Oxyandy
Copy link
Author

Oxyandy commented Apr 17, 2018

Are these all related #330 #673 #674 #675 #924 ?
I have tried to reduce the amount of information on this new issue,
but for more reading there is more detailed info here #1216

@Oxyandy
Copy link
Author

Oxyandy commented Apr 18, 2018

ESP_Easy_mega-20180402_normal_ESP8266_1024.bin
ESP_Easy_mega-20180403_normal_ESP8266_1024.bin
yep

@TD-er
Copy link
Member

TD-er commented Apr 18, 2018

Do you also use MQTT importer? (P037)

@TD-er
Copy link
Member

TD-er commented Apr 18, 2018

Just to be clear, those builds with a date stamp in them are way later than dev-10 and dev-11.

So are we on the same topic again, or is this a completely different topic?
You lost me here.

@TD-er
Copy link
Member

TD-er commented Apr 18, 2018

Then you have probably a different version of this library installed.
I remember to have changed these library calls to min() and max().
Perhaps this was after feb 12, I guess

You can also disable (remove) the plugins calling this lib.

@TD-er
Copy link
Member

TD-er commented Apr 18, 2018

I guess you should go to sleep now, as should I....
And that's saying something, since we're both antipodal to each other. :)
Tomorrow is going to be a very busy day for me, so I guess I will not be looking into your issue before Friday.

@TD-er
Copy link
Member

TD-er commented Apr 19, 2018

One tip, you can cite sourcecode from a specific branch.
The text you quoted does no longer exist in Controller.ino, so the context is hard to see without finding the version etc.

For example:

ESPEasy/src/Controller.ino

Lines 157 to 174 in 40eb3e7

if (MQTTresult)
{
MQTTclient_should_reconnect = false;
log = F("MQTT : Connected to broker with client ID: ");
log += clientid;
addLog(LOG_LEVEL_INFO, log);
subscribeTo = ControllerSettings.Subscribe;
subscribeTo.replace(F("%sysname%"), Settings.Name);
MQTTclient.subscribe(subscribeTo.c_str());
log = F("Subscribed to: ");
log += subscribeTo;
addLog(LOG_LEVEL_INFO, log);
MQTTclient.publish(LWTTopic.c_str(), "Connected");
statusLED(true);
return; // end loop if succesfull
}

@TD-er
Copy link
Member

TD-er commented Apr 19, 2018

I think I have to take a proper look at MQTT, as well as the controller as the importer.
It is just a bit funky how it's working now.

And about the link to source-code. Browse through the code on Github, select the right branch (or tag for this project) and click on a line number.
Then hold shift and select the last line to select.
Then on the [...] and select "use permalink" (or how it's called)
Just paste such a link in a topic here and press preview.

@TD-er
Copy link
Member

TD-er commented Apr 19, 2018

10.49 PM?

@TD-er
Copy link
Member

TD-er commented Apr 29, 2018

I think splitting the bugreports is best, just to get a good summary.

@Oxyandy
Copy link
Author

Oxyandy commented Apr 29, 2018

Oh my, I found the GPIO wrong state cause...
More on that later..
In the interest of trim & tidy I have begun to split the 2 issues, as I result I have deleted everything about the second bug... as that is all going to a second issue, if you like bug history minimal, you may wish to delete your now fragmented replies, sorry

@TD-er
Copy link
Member

TD-er commented Apr 29, 2018

I'm very curious about your findings :)
At the very least, no-one may accuse you of not preparing a bug report ;)

@Oxyandy
Copy link
Author

Oxyandy commented Apr 29, 2018

Ok, so I just flashed ESP_Easy_mega-20180429_normal_ESP8266_1024.bin onto a blank node
Get Wifi setup via AP then go into controllers I see

step_1

I only want to setup Domoticz MQTT, so I just ignore the first controller Domoticz HTTP, after all it's disabled right ? So just edit controller 2 & set that up as Domoticz MQTT, enter IP & submit - Done !

step_2

No problems right ?
@Grovkillen have you ever done this, but with a different controller..
Left the existing first entry Domoticz HTTP ?
Set up the second with what you need ?

@TD-er
Copy link
Member

TD-er commented Apr 29, 2018

There used to be a limitation on the MQTT controllers.
A MQTT controller could only be the first one.

I changed this a while back, to try and make it possible to have multiple MQTT controllers present.
Currently only one MQTT controller may be active and for some functions only the data of the first enabled MQTT controller will be used.

Your setup as shown in the screenshots should be a valid one, but I hope all calls to controllers will check for the 'enabled' checkmark.

@Grovkillen
Copy link
Member

@Oxyandy Yes I've done that, it's a 50/50 thing for me 😉 Shouldn't be any problem.

@TD-er we would not have the Domoticz HTTP as a default controller, leave it blank as the other two.

@Oxyandy
Copy link
Author

Oxyandy commented Apr 29, 2018

Great I am so glad you said "Yes I've done that, it's a 50/50 thing for me"
Cause it is a problem, well "confirmed" for me... I would need to test with your controller though.
What do you use ? Thanks
An explanation will come later, but for me it is a part of the cause of the wrong GPIO state bug.
(I will write it up tomorrow - while your side of the globe sleeps)
And your second comment saves me from saying that too,
it should not be showing Domoticz HTTP on a fresh flash, all controllers should be blank.
I believe it was put there for a reason as a temp fix, that never was improved.
Dates back to Dev_11

@Grovkillen
Copy link
Member

I'm using openHAB MQTT on the second controller.

@Oxyandy
Copy link
Author

Oxyandy commented Apr 29, 2018

The other 'big' reason I was glad you said you have left it 'as is' and used the second
was that I was feeling a little stupid for leaving it there too,
so hearing you've done it as well - makes me feel suddenly 'smarter', haha
Anyway moving on..
I was hoping to find a repeatable scenario where this happened in Grokillen's usage,
I couldn't last night - was falling asleep, I didn't want to wake up with the reverse imprint of letters on my face & forehead so crashed in bed instead..
TD-er I know what causes this now, but the real fix is beyond me
But I can point to the area in the code it happens
and yeah it seems the red X 'disable' is ignored with Domoticz HTTP,
it just needs to be there either 1st or 2nd controller
I do not even know what "Domoticz HTTP" does ? (look more later)
Last night while testing to see if bug exists in 0429, I couldn't repeat it - Until I worked out what I just mentioned above, still happens in 0429, let's fix it.. more later, have work to do

@Oxyandy
Copy link
Author

Oxyandy commented Apr 30, 2018

Dev-10 works, Dev-11 this was added, how does it break GPIO state ?
Commenting the displayed lines out on 0429's source
makes a firmware which I am sick of trying to get a stuck / wrong state
This is not a proper fix, but it does solve the problem

  • but it seems so does editing Domoticz HTTP controller to become Domoticz MQTT,
    so Domoticz HTTP isn't showing in devices even as "disabled"

OLD

case PLUGIN_WEBFORM_LOAD:
{
// We need the index of the controller we are: 0-CONTROLLER_MAX
byte controllerNr = 0;
for (byte i=0; i < CONTROLLER_MAX; i++)
{
if (Settings.Protocol[i] == CPLUGIN_ID_002) { controllerNr = i; }
}
string += F("<TR><TD>IDX:<TD>");
String id = F("TDID"); //="taskdeviceid"
id += controllerNr + 1;
addNumericBox(string, id, Settings.TaskDeviceID[controllerNr][event->TaskIndex], 0, 9999);
success = true;
break;
}

CURRENT

case PLUGIN_WEBFORM_LOAD:
{
// We need the index of the controller we are: 0-CONTROLLER_MAX
byte controllerNr = 0;
for (byte i=0; i < CONTROLLER_MAX; i++)
{
// if (Settings.Protocol[i] == CPLUGIN_ID_002) { controllerNr = i; } -> error: 'CPLUGIN_ID_002' was not declared in this scope
if (Settings.Protocol[i] == 2) { controllerNr = i; }
}
addHtml(F("<TR><TD>IDX:<TD>"));
String id = F("TDID"); //="taskdeviceid"
id += controllerNr + 1;
addNumericBox(id, Settings.TaskDeviceID[controllerNr][event->TaskIndex], 0, 9999);
success = true;
break;
}

@TD-er
Copy link
Member

TD-er commented Apr 30, 2018

It is a very strange change.
It is 'fixing' something by not fixing it.
The protocol was compared with a plugin ID.
And now it is compared with a fixed number, which will break is the order of protocols is ever changed.

And my first impression is the controllerNr should be initialized with CONTROLLER_MAX, which can be considered as "not found".

I have to look at the code a bit longer to get it, what's happening here.

@Oxyandy
Copy link
Author

Oxyandy commented Apr 30, 2018

Yeah, the Domoticz code is a mess
in _C002.ino has references like // temp solution, if plugin 029, set gpio
Have a fresh firmware show 3 "not yet configured" controllers
The disabled controller still isnt really disabled
No delete button in controllers, is it needed ?(OK not needed - set to stand alone)

@Grovkillen
Copy link
Member

Set the first on to "stand alone", that's equal to none/delete.

@Oxyandy
Copy link
Author

Oxyandy commented Apr 30, 2018

Oh yeah,,, of course, that would work, just I have never considered removing a controller until now..
In your case I wonder how things work, no Domoticz HTTP in first - no wrong state ?

@Grovkillen
Copy link
Member

I haven't tested that, maybe? Need to test when I get around to it.

@Oxyandy
Copy link
Author

Oxyandy commented Apr 30, 2018

So this whole issue could have been avoided for me by simply doing things slightly differently..
Anyhow I am pleased I reached the point I know how to avoid it. ;)
Maybe will save others grief one day & makes a better ESpeasy for the future..
Looking forward to a close for this..
Now to complete the New Issue - "Excessive Reporting", cause I have a better understanding of that too.

@Domosapiens
Copy link

Domosapiens commented May 1, 2018

no Domoticz HTTP in first

Same experience here when I extended version 10 to 24 tasks.
Seems to me related to this comment from Hexenmeister:

https://github.com/letscontrolit/ESPEasy/issues/590#issuecomment-344511468
Task-data that overwrites Controller-data because of non-related fixed block sizes.
Hexenmeisters solution relates them to each other.

Grovkillen commented on Nov 15, 2017:

Not in 2.0.0 since we're trying to close the version asap. But I'll add it to 2.1.0 :)

@Oxyandy
Copy link
Author

Oxyandy commented May 2, 2018

Nice, thanks for your input, I will read more later : #590 (comment)

Config.dat is binary, is there an easy way to convert that to plain text ?

@TD-er
Copy link
Member

TD-er commented May 2, 2018

Nope, not yet.
The idea is to get some json import/export later.

@Grovkillen
Copy link
Member

Is this still an issue?

@Grovkillen
Copy link
Member

I will close this, open if its still a valid issue.

Broken Sensors automation moved this from To do to Done Oct 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Plugin Related to supported sensors Type: Bug Considered a bug
Projects
Development

No branches or pull requests

4 participants