stefanbode / Sonoff-Tasmota

Provide ESP8266 based itead Sonoff with Web, MQTT and OTA firmware using Arduino IDE, enhanced with I2C options
GNU General Public License v3.0
127 stars 41 forks source link

PID control #87

Closed colinl closed 4 years ago

colinl commented 5 years ago

Hi @stefanbode you asked me to raise an issue here related to incorporation with the PID algorithm. See https://github.com/colinl/Sonoff-Tasmota/issues/8 for orginal discussion.

colinl commented 5 years ago

To answer your question in the other thread: "Where do I find your PID enhancements in the code?" There aren't any changes required to the tasmotta code. All that is necessary is to pick up, from my branch [1], the files lib/ProcessControl/PID.cpp and PID.h and the file sonoff/xdfv_92_pid.ino and include them in the source to be built. Then instructions for setting it up are included in xdrv_92_pid. Additional usage instructions are in the wiki page on pid control [2]

[1] https://github.com/colinl/Sonoff-Tasmota/tree/pid_branch. [2] https://github.com/arendst/Sonoff-Tasmota/wiki/PID-Control-with-Sonoff-Devices

ascillato commented 5 years ago

Hi, I'm also interested on this. PID from @colinl is working great!

stefanbode commented 5 years ago

Weekend ahead. Please be patient. There is an enhancement required that multiple PID can be used not only one.

colinl commented 5 years ago

Referring back to the other thread again, to generate a Pull Request against some else's code you should fork the repository then start a branch at the relevant point. So if you were planning to extend my code you should fork my repo then go to my pid branch and start a new branch there. Make all your changes on your new branch. Once that is complete you can generate a PR of the mods on your branch which I can then merge onto mine.

ascillato commented 5 years ago

This is super useful, so the PR can be made also to arendst/tasmota if you agree

colinl commented 5 years ago

If you are adding multi loop capability then ideally all the mods should be conditionally compiled so that if multi loop is not enable it performs exactly as currently. Otherwise picking up the latest would break existing systems. I am very happy for it to be incorporated in the main repo, though I would prefer it to be merged into mine first if that is ok. I did offer to do that some time ago I believe but I don't think it was met with any great enthusiasm. My memory of exactly what happened may be faulty though. I can't immediately find the conversation.

By the way have you seen this https://github.com/arendst/Sonoff-Tasmota/issues/288

thucar commented 5 years ago

Now a new question arises for @colinl - is the direct usage of the power value as valve position, the way I did in my initial test, the best approach? It has worked for me in my node-red setup but I can see use cases where a cap might need to be added, to limit the valve travel. For instance the temperature rise in my hot water system is not in correlation with the movement of the valve. For the first 25% of the travel my output temperature hardly changes at all. Then the majority of the change takes place in the 25 - 50% range. After that it tapers off again.

I have been scaling the power value from 0-1 to 30 - 70 to smooth things out. This has been working well for me. Maybe it would be beneficial to add scaling options to this as well?

colinl commented 5 years ago

Did you try without first? PID control is very good at ironing out non-linearities in the control system.

stefanbode commented 5 years ago

Hi @thucar . You have a similar problem already with the shutter blinds. The real position is not linear with the motor runtime. I already prepared , but not set active the ability to calibrate the complete setup with a polynomial of degree 2 (ax^2+bx+c=y) instead of ax=y. For shutters this is more or less the exact behaviour. For valves,e.g. ball valves this is a completely different game. Anyhow I prefer to correct this at the shutter/valve side. The PID controller cannot do anything. Another very very important topic is that the valve opreration time cannot be indefinite short. For shutters it looks like 0.5sec is minimum to avoid a shift of the position

colinl commented 5 years ago

@stefanbode on the minimum travel issue, when I coded a similar algorithm many years ago in a previous life I think I included a parameter dead_time in the setup parameters. This should be the time taken to actually get the motor going. Then when I worked out the time to switch the motor on you can just add on the dead_time to get a total time. This automatically meant that the minimum On time was the dead_time. On the scaling, yes I agree, the scaling should be applied at the shutter/valve side.

thucar commented 5 years ago

@colinl I was running the node-red version for about 6 months without any scaling. It did work and it was not too bad. The only times I really felt it and what made me look into scaling, was when in the middle of the winter, the room was up to temperature and the valve was almost completely closed. When the radiators were no longer radiating heat, it felt chilly even though the room temperature had been achieved. It took about half an hour before the valve was open enough (at least 30%) that the radiators were starting to radiate heat once again.

After adding scaling, the issue was gone.

stefanbode commented 5 years ago

hi, nice pice of code. With only some minor changes I was able to control the shutter with temperature. In the future I will make the source flexible for the 4 PID slots and will persist the changed configuration parameters in settings. The initial functionality is now running with this build.

colinl commented 5 years ago

Excellent. With a thought to the eventual PR make sure that on the branch you only make changes to the PID code. If you have other stuff that also needs to go in then you can start another branch from the tip of the PID branch and each time you make changes to the PID code you rebase the other branch to that so that it stays on the tip. I haven't looked at your shutter code, but have you considered using the xdrvnn feature (if you haven't already done that) so that it is a drop in feature like the PID and doesn't affect the base code? That makes it much easier to keep it up to date with the base code. That is how I did my time proportioned output which I guess is rather similar. If you wanted to see how I did it then it just consists of the file sonoff/xdrv_91_timeprop.ino on the pid branch in my repo.

colinl commented 5 years ago

Thinking about the conditional compilation issue I suppose what matters is that if only one loop is wanted then the external interface via mqtt and commands should stay the same, and the #defines that are already defined still work as they currently do. So if you specify a new define, something like # define PID_LOOPS n where n is number of loops then at compile time it can test for that defined and if it is carry on, if it isn't then use the old defines to setup all the new ones. Something like that anyway

thucar commented 5 years ago

Is this still a thing or has it been shelved for the time being? This would be a huge upgrade for all sorts of heating and lighting automation solutions if we could bring these two together.

stefanbode commented 5 years ago

Currently there in one PID slot available as in the original shared code. This should work. Are you requesting the 4 slot solution?

thucar commented 5 years ago

I'm requesting a merge of the codes :) So I could have the latest, up to date version of Tasmota and just enable shutter control and/or PID control as needed on a case by case basis.

With PID control I already kinda have that, because I can grab the latest Tasmota and just drop in the required couple files. With shutter control I'm limited to using this repo as a base.

colinl commented 5 years ago

I think the first thing to do would be to convert the shutter control to use the same technique for inclusion as the PID does, so then it be just a matter of including the files in the build. Then I could easily make the changes to the PID to use the shutter control. When I first did the PID I enquired whether there would be interest in a PR but got no response, if I remember correctly.

thucar commented 5 years ago

@colinl topics seem to get closed off pretty fast on that repo and as such, they fall out of sight at least for casual browsers. Not sure if contributors go over closed topics on a regular basis.

But at the moment I see @ascillato - a contributor over at Tasmota has stated above that he believes a PR would be a good idea.

colinl commented 5 years ago

@thucar have you got a link to where @ascillato has said that?

thucar commented 5 years ago

Sure - its' right here: https://github.com/stefanbode/Sonoff-Tasmota/issues/87#issuecomment-437336465

ascillato commented 5 years ago

I agree, PID is super useful. I want to add it to main Tasmota but I want to make it easier to choose the sensor (input for PID) at runtime. I haven't had the time but I was thinking on just adding the JSON name of the sensor as a pointer for the PID console command (similar like used in rules features).

I wanted to make a PR with a PoC (proof of concept / example) of soft controlling a light brigthness with a LDR sensor as an energy saving feature. If there is natural light in the room, the light is dimmed. If there isn't, the light is brighter mantaining the same amount of lux on that room.

Besides PID, also I would love to see shutters support in main Tasmota.

Theo always wants that any new feature be in a separated driver file and that it modify as little as possible of the rest of the files.

At this moment I'm finishing a project at work, so I have very little free time, but I would love to help on this two features :+1:

All you have done an amazing job with those!

colinl commented 5 years ago

It would be great to see the PID included. I agree some changes need to be made in order for that to happen. My problem is that I know about process control (PID etc) but I don't know much about how the Tasmota s/w works under the hood so it isn't easy for me to make the changes. Also I have very little time available at the moment. However if someone who knows about the internals was available to do that work then that would be great.

stefanbode commented 4 years ago

Pid is now part of the main code an can be added. No need anymore here

colinl commented 4 years ago

I can't find it in the main repository, can you give me a clue as to where to find it?

colinl commented 4 years ago

When you said it is now part of the main code did you mean this repository, or that it has been added to the arendst repository?

stefanbode commented 4 years ago

Ate a look here https://github.com/arendst/Sonoff-Tasmota/wiki/Time-Proportioned-Output-support

colinl commented 4 years ago

That is referring to my fork, so I still don't understand what was meant by "Pid is now part of the main code an can be added. No need anymore here"