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

Does not compile with tasmota 9.2.0 #15

Open
marcvs opened this issue Jan 4, 2021 · 9 comments
Open

Does not compile with tasmota 9.2.0 #15

marcvs opened this issue Jan 4, 2021 · 9 comments

Comments

@marcvs
Copy link

marcvs commented Jan 4, 2021

Hi There,

this does not compile with 9.2.0 any longer.

In fact I had some functional issues with 8.2.0 already.

I'd go an adapt the code to work with 9.2.0 and seek advice for fixing functionality.

Objections?

Shall I MR against this branch or against arendst/Sonoff-Tasmota?

Cheers,
M.

@colinl
Copy link
Owner

colinl commented Jan 4, 2021

A PR here against PID_branch and/or timeprop_branch would be most welcome, thanks.
Preferably only changing the files already modified on those branches if possible.

@marcvs
Copy link
Author

marcvs commented Jan 4, 2021

Hmm, but compiling against latest development would not work here, right?

For the moment I'm trying to get it working with Tasmota/development. I can PR the new files back here, if this is what you say.

@colinl
Copy link
Owner

colinl commented Jan 4, 2021

I will update the repo to pull in the latest from arendst. Hopefully a bit later today if I can find time.

@ascillato
Copy link

After that, please, also do a PR to the development branch of Tasmota. I would like to see PID also in the main branch. Thanks.

@marcvs
Copy link
Author

marcvs commented Jan 4, 2021

Alright I'm almost there.

I don't like the Hardcoded DS18B20 values, that I found in the original code. This is probably a showstopper.

Also, I had to make a copy of the TasmotaGlobal.mqtt_data before feeding it to the JsonParser.

Regarding the PR: Are there any instructions for building with the required platforms via platformio?

@colinl
Copy link
Owner

colinl commented Jan 4, 2021

I don't understand, there isn't anything DS18B20 specific in the modified files in the pid or timeprop branches, you should only be editing sonoff/xdrv_92.ino and sonoff_91.xdrv_91 to get them to build with 9.2.0

@marcvs
Copy link
Author

marcvs commented Jan 4, 2021

Correct. I only modified those two.

The DS18B20 stuff comes in, because at some point I need to pass a sensor value to pid.setPv.
This is currently obtained from TasmotaGlobal.mqtt_data searching for "DS18B20" and there "Temperature".

This is fine for me, but I'm not sure if this is "tasmotic" enough. Also I'm not sure how this could / should be configured (pointers welcome, but I'll not have time for this after this weekend).

@colinl
Copy link
Owner

colinl commented Jan 4, 2021

Is this anything to do with the issue here, and a possible PR for the timeprop files, to fix the fact that the pid code won't compile with 9.2.0?

@marcvs
Copy link
Author

marcvs commented Jan 4, 2021

No, the additional points I rose, were merely to highlight those parts of the code that I thought one should be aware of.
Regarding the issue, my changes will close it and not change the behaviour in general.

I'll send the PR in a sec.

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

No branches or pull requests

3 participants