search

MichielVanwelsenaere / HomeAutomation.CoDeSys3

Home Automation system build in CoDeSys 3 with MQTT communication to any third party Home Automation software
MIT License
109 stars 36 forks source link

FB_OUTPUT_COVER_MQTT: replacing implementation to a more stable one #137

Closed MichielVanwelsenaere closed 1 year ago

MichielVanwelsenaere commented 1 year ago

Pull Request Checklist

Check if the tasks below are relevant for your PR, to mark an item as completed use [X].

Proposed Changes

Refactors the cover function block. No longer using position as a primary value to steer the cover but switching to time. The calculated position proved to be unreliable. Granular position control has -for the time being- been dropped in favor of improved stability. It might be added again over time. MQTT Discovery has been adopted accordingly.

alegsan commented 1 year ago

Hi Michiel,

i'am following your project for some time and now are trying to port my current implementation to yours. Thank you for sharing your project to the community :thumbsup: .

In our house we have a spcieal up and down button for the covers. With this PR you have switched to a one-button operation. Could you please explain what the exact problem was with the previous implementation and if you have any thoughts as to the cause of the problem? I would try to solve the problem you were encountered.

MichielVanwelsenaere commented 1 year ago

Hi Oleg,

the nature of the problem was not related to the up/down buttons. The issue was that the previous implementation calculated a position of the cover and used that variable for deciding if a cover was allowed to go up/down further or not. From the moment the position variable was no longer accurate the cover couldn't move up any further up or down because it was already on a 0 or 100% position.

After giving it some thougth I switched to a time driven approach, if you move the cover completely up now it will just power the motor for the maximum amound of time (a variable you can configure) in that direction. This approach simplifies things considerably resulting in an improved stability. I've been using this implementation for over a year now and haven't had any issues.

Positional control got dropped as a result but it can theoritically be added again as long as it's just calculated from the time and not used a variable for control. Something I might experiment with in the future when I have some time again (speaking 1y+ here).

Now, to give a short and simple answer on your question; simple up/down support got dropped during some refactoring of the function block for improved stability but can actually easily be added again. I think the only thing you need to do is:

  1. add the UP/DOWN inputs
  2. add 2x rising edge detection (one for each up/down)
  3. if the rising edge detection detects a request on either up/down just encorperate it in the logic of the FB. I should be very similar to what is already there for the TOGGLE input.

If you're having trouble during implementation; feel free to reach out. If you're not up for the task but would be able to test let me know as well. I'll see if I can cook something up for you. I don't use this functionality myself and prefer to only add things in the project that are properly tested. (= the reason the up/down inputs got dropped :) ) If you've got something working please reach out so we can add it here

alegsan commented 1 year ago

Hi Oleg,

Hi Michiel,

thank you for explaining the problem.

the nature of the problem was not related to the up/down buttons. The issue was that the previous implementation calculated a position of the cover and used that variable for deciding if a cover was allowed to go up/down further or not. From the moment the position variable was no longer accurate the cover couldn't move up any further up or down because it was already on a 0 or 100% position.

In you previous implementation you have used the function block BLIND_ACTUATOR from the oscat library. Is your experienced problem located in the implementation of this function block? In my current PLC code iam using the function block BLIND_CONTROL_S. This function block has some more options to handle this situations you described like T_EXT. It also let you define time definitions for up and down separatly. Did you tried this function block in the past?

If you're not up for the task but would be able to test let me know as well. I'll see if I can cook something up for you. I don't use this functionality myself and prefer to only add things in the project that are properly tested. (= the reason the up/down inputs got dropped :) )

I will try to reimplement the cover implementation but i can also test any changes from you :-).

If you've got something working please reach out so we can add it here

:thumbsup:

MichielVanwelsenaere commented 1 year ago

In you previous implementation you have used the function block BLIND_ACTUATOR from the oscat library. Is your experienced problem located in the implementation of this function block? In my current PLC code iam using the function block BLIND_CONTROL_S. This function block has some more options to handle this situations you described like T_EXT. It also let you define time definitions for up and down separatly. Did you tried this function block in the past?

I have not so knock yourself out :). Thanks!