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

Cover is driving all the way up or down until end position, if a small position correction is needed #103

Open
JSchmid6 opened this issue May 20, 2024 · 12 comments
Assignees
Labels
bug Something isn't working

Comments

@JSchmid6
Copy link
Contributor

I would like to suggest some modifications I have made to your integration and in more detail to the covers part. The goal of these changes is to prevent the covers from moving all the way up or down when only a small adjustment is needed.

I have adjusted the calculations in cover.py as follows:

Original:
time = min(int(((position - self._attr_current_cover_position) / 100.0) * self._time_opens), 255)

Changed to:
time = max(1, min(int((float(position - self._attr_current_cover_position) * float(self._time_opens) / 100.0)), 255))
and
Original:
time = min(int(((self._attr_current_cover_position - position) / 100.0) * self._time_closes), 255)

Changed to:
time = max(1, min(int((float(self._attr_current_cover_position - position) * float(self._time_closes) / 100.0)), 255))

Additionally, I modified lines 183 and 186 to ensure that the covers will move to the end position when 100 or 0 is set. This helps with calibration if the device is out of sync.

Original:

elif position == 100:
    direction = "up"
    time = self._time_opens + 1
elif position == 0:
    direction = "down"
    time = self._time_closes + 1

Changed to:

elif position == 100:
    direction = "up"
    time = 0  # self._time_opens + 1
elif position == 0:
    direction = "down"
    time = 0  # self._time_closes + 1

It would be great if you could incorporate these changes into your integration.
Thank you very much for your excellent work and great commitment.

@JSchmid6
Copy link
Contributor Author

JSchmid6 commented Jun 2, 2024

Could you please take a look at this? It's important to me. I have to edit it each time I update it. What do you think about these changes?

@grimmpp grimmpp added bug Something isn't working labels Jun 5, 2024
@grimmpp
Copy link
Owner

grimmpp commented Jun 5, 2024

Hello @JSchmid6,
thanks for your contribution. Unfortunately, I don't have a proper testing environment yet. It's under development ;-). And I'm wondering if your fix would work for everyone.
Would it work in your case to just increase the time_opens and time_closes in the configuration?
if not I would rather prefer a global parameter in the configuration you can choose so that you can adjust it independently to each cover brand.

@JSchmid6
Copy link
Contributor Author

JSchmid6 commented Jun 6, 2024

Hello grimmpp,
You are right, there is a risk that the calibration might not work for every device. I have already considered this, and since it is not critical, we can omit this part of the change. I have created a pull request without that change. A step of 0 is the real issue. It would be great if we could integrate a minimal step of 1, which should work for every device.

@mkaufmann
Copy link

I agree with the pull request in general. I'm wondering though why second granularity path was taken. In my previous - less well integrated - own enocean integration I used the 100ms granularity setup. Would you be fine if we switch to that approach? It would allow to more accurately do small corrections

@JSchmid6
Copy link
Contributor Author

JSchmid6 commented Jul 2, 2024

Of course, if 100ms are possible I would be happy. I think I missed that. For me it looked like the smallest number, to be set is one.

@grimmpp
Copy link
Owner

grimmpp commented Jul 3, 2024

@mkaufmann can you tell us what to adapt? In the meantime I will takeover this change into feature-branch and test it.

@grimmpp grimmpp self-assigned this Jul 3, 2024
@grimmpp
Copy link
Owner

grimmpp commented Jul 3, 2024

I've merged it now into feature-branch. Can you please test it?

@mkaufmann
Copy link

In the 3F_7F telegram as documented here for FSB14 https://www.eltako.com/fileadmin/downloads/en/_main_catalogue/Gesamt-Katalog_ChT_gb_highRes.pdf there is a DB0_Bit1 that allows to configure 100ms granularity instead of second granularity.
The caveat is that one has to use both data bytes 2 and 3 for runtimes > 25.5 seconds in that mode.

This is the Python code that I had running in my old script for computing the msb & lsb byte fields

    duration_100ms = int(duration * 10)
    duration_msb = 0 if duration_100ms < 255 else int(duration_100ms / 255)
    duration_lsb = (duration_100ms % 255) if duration_100ms >= 255 else duration_100ms

and this was the code for getting the duration from the response

        duration = (packet.data[1]*256 + packet.data[2]) / 10.0

I hope this makes sense.

@grimmpp
Copy link
Owner

grimmpp commented Jul 4, 2024

Funnily, the command is implemented but the one with seconds is used.
https://github.com/grimmpp/eltako14bus/blob/38ceae7a068c46905c5735974a83f860dd26c95c/eltakobus/eep.py#L1100

But only used for receiving the status not for sending the command.

msg = H5_3F_7F(time, command, 1).encode_message(address)

Maybe you can try to replace the line with: msg = G5_3F_7F(None, time, command).encode_message(address)

@mkaufmann
Copy link

mkaufmann commented Jul 7, 2024

The full close on short time frames is fixed, can confirm. With the H5 to G5 change I encountered some problems and need to debug more

@JSchmid6
Copy link
Contributor Author

I've merged it now into feature-branch. Can you please test it?

I tried to use the feature branch, but I got some problems with tilt position. Is that something new? How could I test this branch? What do I have to do? Thanks for your support

@grimmpp
Copy link
Owner

grimmpp commented Jul 21, 2024

Yes, there are changes in it. You can see them here.

What problems did you discover?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants