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

NEC one/zero codes reversed #40

Closed
kevinjwalters opened this issue Nov 5, 2020 · 4 comments · Fixed by #68
Closed

NEC one/zero codes reversed #40

kevinjwalters opened this issue Nov 5, 2020 · 4 comments · Fixed by #68

Comments

@kevinjwalters
Copy link

kevinjwalters commented Nov 5, 2020

I'll split this up as appropriate after some discussion, I just wanted to record some things I just noticed:

  1. NEC one/zero look reversed in
    header=[9500, 4500], one=[550, 550], zero=[550, 1700], trail=0

    For comparison there's an LIRC configuration file shown in Adafruit Learn: Using an IR Remote with a Raspberry Pi Media Center: Configure and Test which shows the one represented with 594 1634, the zero by 594 519.
    That won't matter if the data isn't used outside of this library but it will break and confuse any attempts at IR between CP and all other code / IR data. The reversed values and data feature elsewhere too, e.g. Adafruit Learn: Infrared Receive and Transmit with Circuit Playground Express.

Other observations.

  1. The duty cycle on the carrier is 50% in examples. I noticed an analysis of Sony's IR output and the claim was that was 20%. That could be because it's more power efficient and perhaps that's useful on the CPX as well to reduce power usage?
  2. The trail=0 appears to break my attempt to generate Sony IR codes. I just tested a feature to support trail=None and my code now works. What does setting a final pulse (using pulseio) of 0 on the end of a load of duration pairs do?
  3. Sending sony codes can require sending 20 bits (not an integer number of bytes). I've just coded up an optional keyword arg to support this.
@kevinjwalters
Copy link
Author

kevinjwalters commented Mar 28, 2021

Observations 2 and 3 are covered by #44 #46

@kevinjwalters kevinjwalters changed the title Misc possible issues NEC one/zero codes reversed Apr 9, 2021
@tekktrik tekktrik self-assigned this Jan 18, 2023
@tekktrik tekktrik removed their assignment May 30, 2023
@Kheirlb
Copy link

Kheirlb commented Oct 4, 2023

Just hit this issue myself: flipping one/zero resolved the issue for me.

Thanks for creating the issue @kevinjwalters, hopefully it can be fixed soon-ish by the CircuitPython team? I can also trying opening up a PR.

@tannewt
Copy link
Member

tannewt commented Oct 4, 2023

Thanks for creating the issue @kevinjwalters, hopefully it can be fixed soon-ish by the CircuitPython team? I can also trying opening up a PR.

PR would be great! Thanks!

@tkomde
Copy link
Contributor

tkomde commented Apr 9, 2024

@kevinjwalters @tannewt

Hi, I appreciate the work of the CircuitPythot team on a daily basis.
I struggled with this for a day, and I think there are two problems.
(I have verified both sending and receiving.)

(1) Bit is inverted when judging

Due to this site, bit is defined as follows.

  • Logical '0' – a 562.5µs pulse burst followed by a 562.5µs space, with a total transmit time of 1.125ms
  • Logical '1' – a 562.5µs pulse burst followed by a 1.6875ms space, with a total transmit time of 2.25ms

Current decode logic is...

    # convert marks/spaces to 0 and 1
    for i, pulse_length in enumerate(pulses):
        if (space * 0.75) <= pulse_length <= (space * 1.25):
            pulses[i] = False
        elif (mark * 0.75) <= pulse_length <= (mark * 1.25):
            pulses[i] = True

So, this should be…

    # convert marks/spaces to 0 and 1
    for i, pulse_length in enumerate(pulses):
        if (space * 0.75) <= pulse_length <= (space * 1.25):
            pulses[i] = True
        elif (mark * 0.75) <= pulse_length <= (mark * 1.25):
            pulses[i] = False

By this change, TX and RX will be consistent.

The NEC protocol will works fine with this change, but other protocols may be affected. The protocols themselves are many…
https://github.com/Arduino-IRremote/Arduino-IRremote?tab=readme-ov-file#supported-ir-protocols

(2) The sample is not in the NEC protocol

This is the original issue.

# Create an encoder that will take numbers and turn them into NEC IR pulses
encoder = adafruit_irremote.GenericTransmit(
    header=[9500, 4500], one=[550, 550], zero=[550, 1700], trail=0
)

should be…

# Create an encoder that will take numbers and turn them into NEC IR pulses
encoder = adafruit_irremote.GenericTransmit(
    header=[9000, 4500], one=[563, 1690], zero=[563, 563], trail=563
)
  • Reverse 0/1
  • Add trailing burst
  • Trim duration(this change is not mandatory )

I can make a PR, but I think there are two options.

Option1

  • Simply fix these.
  • The existing code will no longer work. However, I did quite a bit of web research on this issue, but it was not searched at all, so the impact may be minimal...

Option2

  • Add option to decode_bits() like “bit_definision -> bool” which inverts bits.
  • Add NECDecode aside GenericDecode and pass bit_definision parameter to decode_bits().
  • Existing code will work, but implementation will be more complex.

Please give me your opinion.

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 a pull request may close this issue.

5 participants