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

add support for nexus protocol to rc-switch library #21886

Merged
merged 2 commits into from
Sep 1, 2024

Conversation

slo617
Copy link
Contributor

@slo617 slo617 commented Aug 1, 2024

Description:

New feature:

  • This adds support for nexus protocol to rc-switch library.

    Nexus protocol is used by temperature and humidity sensor that operate at 433 MHz.
    It is used by various brands.

Checklist:

  • The pull request is done against the latest development branch
  • Only relevant files were touched
  • Only one feature/fix was added per PR and the code change compiles without warnings
  • The code change is tested and works with Tasmota core ESP8266 V.2.7.7
  • The code change is tested and works with Tasmota core ESP32 V.3.0.2
  • I accept the CLA.

@eku
Copy link
Contributor

eku commented Aug 2, 2024

A reference to the protocol description would be nice.

@slo617
Copy link
Contributor Author

slo617 commented Aug 2, 2024

The best documentation I found so far is this one: https://github.com/aquaticus/nexus433
I double checked and compared timings to what I have seen on my scope btw.

Do you wish to have a protocol description or reverence in code or commit message?

@Jason2866
Copy link
Collaborator

Jason2866 commented Aug 3, 2024

@Jason2866
Copy link
Collaborator

Jason2866 commented Aug 3, 2024

@slo617 Is the change of RCSWITCH_SEPARATION_LIMIT needed?
I remember there are side effects when changing.
Found the regarding PR #14421
Honestly reluctant to change this value

@slo617
Copy link
Contributor Author

slo617 commented Aug 3, 2024

Yes, change of RCSWITCH_SEPARATION_LIMIT is needed to use the new protocol.
Old value was 2600ms (3 years ago). I changed from 4100 to 7*500=3500. This is still higher than the old value.
7*500 is the exact length of the 'low' part of the sync signal. (Actually there is only the low-part...)
I can not find that issue #14 mentioned in the comment. I do not know why 4100 was chosen. I guess it was just the next bigger value.
In my tests, everything still seem to work fine.
Anyhow, if there really still is an issue, maybe we should iterate over all protocols that are enabled at run-time and chose the same time as the shortest 'low' part of the sync signal, as suggested by the comment in code.

@Jason2866
Copy link
Collaborator

Jason2866 commented Aug 3, 2024

The value 4100 is choosen because there where issues with the value before (2600).
Reducing to the suggested lower value of 3500 may break implemented protocols.
Sorry no merge, as long it is not verified all currently implemented protocols do still work with this lowered value.
The Tasmota lib is based on the 1technophile build/fork. More infos about the issue(s) can be found there. Starting point: 1technophile/rc-switch#7
Honestly I will not invest a lot of time again in this just because one protocols is added.
Feel free to change the lib / Tasmota to make this value adjustable.

@Jason2866 Jason2866 added the on hold by dev team Result - Feature request put on hold by member of development team label Aug 3, 2024
@slo617
Copy link
Contributor Author

slo617 commented Aug 3, 2024

Sure, got that, but why 4100? How was that calculated? Why not 4000 or 4200?

Anyhow, I took a look into the code once again...
A new (potential) message is detected if there is a pulse with a length of at least RCSWITCH_SEPARATION_LIMIT milliseconds.
So RCSWITCH_SEPARATION_LIMIT must be equal or shorter than the initial pulse, but must not be equal or shorter than any following pulses of the message. Otherwise a new start would be detected within the message.
But as far as I can see, that is still the case for the same protocols it was before my change.

@Jason2866
Copy link
Collaborator

Jason2866 commented Aug 3, 2024

It is a long time ago. Don't remember the details. It is not just calculated. Many real life tests with different values where done. Overall the current choosen 4100 is working quiet well for most.
Again no change without real life tests. -> We have been there with "the calculated value should work fine"

@sfromis
Copy link
Contributor

sfromis commented Aug 4, 2024

That #define RCSWITCH_SEPARATION_LIMIT 4100 could be "wrapped" in #ifdef, allowing it to be a build time option, which you can change in a custom build. While not "ideal", this would make it feasible to use a protocol needing to deviate from the currently chosen default.

Copy link

This PR has been automatically marked as stale because it hasn't any activity in last few weeks. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale Action - Issue left behind - Used by the BOT to call for attention label Aug 29, 2024
Nexus protocol is used by temperature and humidity sensor that operate at 433 MHz.
It is used by various brands.
@github-actions github-actions bot removed the stale Action - Issue left behind - Used by the BOT to call for attention label Sep 1, 2024
@slo617
Copy link
Contributor Author

slo617 commented Sep 1, 2024

I reverted my changes on RCSWITCH_SEPARATION_LIMIT. Instead I added a new function that calculates the correct limit from the values of the activated protocols. It does so by searching for min and max timings and choosing the value in the middle.
If conflicting timings are found it will fall back to RCSWITCH_SEPARATION_LIMIT, which is the case if all protocols are enabled. In that case an info-message is logged.

I tested with all protocols by sending messages from one tasmota device to another tasmota device. (Both are ESP32 in my case.)
In addition to those protocols that had worked before, after my change I got also protocols 4, 16, 17, 18, 23, 25, 26, 27, 29, 32 and 34 working - which had not worked out-of-the-box before.
Protocols 12-14 are marked as "not working" or "test" - I could not get them working before and after my changes. Same with Protocol 15 which has no documentation.
Protocols 11, 14, 15, 37 have a header of 0 (zero) pulses and will keep using RCSWITCH_SEPARATION_LIMIT. (And I could not manage to send and receive them with my test setup.)
Same for Protocols 19 and 31 which "data pulses" that are longer than "header pulses". These will also keep unsing RCSWITCH_SEPARATION_LIMIT, and I am not sure if receiving these protocol with rc-switch is possible at all. (But sending should be possible of cause.)
In addition protocols 24, 28, 30, 33, 35, 36 have not worked before and after my change. Protocol 24 has no transition between two set or two cleared bits, not sure if it can be received at all.

I also fine-tuned the new protocol a bit, so that sending is working too.

@Jason2866
Copy link
Collaborator

like this approach. Imho worth to give it a try. @arendst What do you think about?

@Jason2866 Jason2866 removed the on hold by dev team Result - Feature request put on hold by member of development team label Sep 1, 2024
@arendst arendst merged commit caa501b into arendst:development Sep 1, 2024
59 checks passed
hawa-lc4 pushed a commit to hawa-lc4/Tasmota-dev that referenced this pull request Sep 16, 2024
* add support for nexus protocol to RCSwitch library

Nexus protocol is used by temperature and humidity sensor that operate at 433 MHz.
It is used by various brands.

* calc separation limit for RCSwitch library automatically
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

Successfully merging this pull request may close these issues.

5 participants