search

cyborg5 / IRLib

An Arduino library for encoding and decoding infrared remote signals
Other
213 stars 76 forks source link

IRdecodeBase::decodeGeneric doesn't work with IR Commands where the data is in the spaces. #12

Closed regnets closed 9 years ago

regnets commented 9 years ago

I got the following ir command:

define LIGHT_STRIKE_RAW_LENGTH 66

define LIGHT_STRIKE_HEADER_MARK 6750

define LIGHT_STRIKE_HEADER_SPACE 0

define LIGHT_STRIKE_MARK_ONE 750

define LIGHT_STRIKE_MARK_ZERO 750

define LIGHT_STRIKE_SPACE_ONE 3400

define LIGHT_STRIKE_SPACE_ZERO 900

The Problem is, that as long as the parameter Mark_One is set to something else than zero the function goes in the if branch for data in the pulses.

However my ir command has the data in the spaces, but still got marks for one and zero.

I think this is an error in the decodeGeneric function. Please correct me if i am wrong.

In my opinion there should be a parameter switch to tell the function if the data is contained in the pulses or in the spaces.

Here is an example of the rawbuf of my protocol. Raw samples(66): Gap:276 Head: m6600 s900 0:m750 s950 1:m800 s850 2:m800 s850 3:m800 s900
4:m750 s3400 5:m750 s3450 6:m750 s3400 7:m750 s900
8:m800 s900 9:m750 s900 10:m800 s900 11:m750 s900
12:m750 s900 13:m750 s950 14:m750 s900 15:m750 s900

16:m750 s950 17:m750 s900 18:m750 s900 19:m750 s950
20:m750 s3400 21:m750 s950 22:m750 s3400 23:m750 s900
24:m800 s900 25:m750 s900 26:m750 s950 27:m750 s900
28:m750 s900 29:m750 s3450 30:m750 s900 31:m750 Extent=75000 Mark min:750 max:800 Space min:850 max:3450

If this is really a bug, i will provide a pull request for you, but i really would like to talk about that error, i am searching for the mistake for days now.

regnets commented 9 years ago

Ok i already saw that some protocols are using the other if branch in decodegeneric.

Furthermore my protocols is also recognized by decodegeneric, if i set Mark_One to 0.

But that doesn't make any sense to me, i thought that the parameters are representations of the physical pulses and spaces in the protocol.

My Protocol got clearly a mark of 750-800ns for a one bit and if i use sendGeneric i'll have to provide the correct length for the one mark.

So clearly this isn't a real error, but it is really confusing. In my opinion the send and decode function should work more similar.

cyborg5 commented 9 years ago

I'm sorry for the confusion. When I first developed this library based on the IRRemote by KenShirriff it seem like there was a lot of redundant code so I tried to collapse things into generic routines that could be called and controlled completely by their parameters. If you read the comment just before the code for decodeGeneric method in IRLib.cpp it explains how that one parameter is used to designate whether it is a variable Mark variable Space. It saved adding another parameter to the call. Probably not my most brilliant piece of coding :-)

As it turns out Sony is the only protocol I've ever found that use variable marks rather than variable spaces. In the next version of the library I'm going to clean up that routine and make a specialized routine just for Sony. Unfortunately for all of my IR fans out there, I recently purchased a 3-D printer so all of my IR projects are on hold while I play with my new toy.

From: regnets [mailto:notifications@github.com] Sent: Friday, August 14, 2015 12:42 PM To: cyborg5/IRLib IRLib@noreply.github.com Subject: [IRLib] IRdecodeBase::decodeGeneric doesn't work with IR Commands where the data is in the spaces. (#12)

I got the following ir command:

define LIGHT_STRIKE_RAW_LENGTH 66

define LIGHT_STRIKE_HEADER_MARK 6750

define LIGHT_STRIKE_HEADER_SPACE 0

define LIGHT_STRIKE_MARK_ONE 750

define LIGHT_STRIKE_MARK_ZERO 750

define LIGHT_STRIKE_SPACE_ONE 3400

define LIGHT_STRIKE_SPACE_ZERO 900

The Problem is, that as long as the parameter Mark_One is set to something else than zero the function goes in the if branch for data in the pulses.

However my ir command has the data in the spaces, but still got marks for one and zero.

I think this is an error in the decodeGeneric function. Please correct me if i am wrong.

In my opinion there should be a parameter switch to tell the function if the data is contained in the pulses or in the spaces.

Here is an example of the rawbuf of my protocol. Raw samples(66): Gap:276 Head: m6600 s900 0:m750 s950 1:m800 s850 2:m800 s850 3:m800 s900

4:m750 s3400 5:m750 s3450 6:m750 s3400 7:m750 s900

8:m800 s900 9:m750 s900 10:m800 s900 11:m750 s900

12:m750 s900 13:m750 s950 14:m750 s900 15:m750 s900

16:m750 s950 17:m750 s900 18:m750 s900 19:m750 s950

20:m750 s3400 21:m750 s950 22:m750 s3400 23:m750 s900

24:m800 s900 25:m750 s900 26:m750 s950 27:m750 s900

28:m750 s900 29:m750 s3450 30:m750 s900 31:m750 Extent=75000 Mark min:750 max:800 Space min:850 max:3450

If this is really a bug, i will provide a pull request for you, but i really would like to talk about that error, i am searching for the mistake for days now.

— Reply to this email directly or view it on GitHub https://github.com/cyborg5/IRLib/issues/12 . https://github.com/notifications/beacon/ADcgugFh3ZMS2pjsAm-ovlGVYic-sRyPks5onhHcgaJpZM4FrynT.gif

regnets commented 9 years ago

If you accept Pull Requests, i can make the changes. I'll just take the "sony" part out of the generic decode method, than i will change all the decodegeneric calls.

cyborg5 commented 9 years ago

I've already made the change in a work in progress version so I'd rather not have two different versions of moving Sony that I would later have to reconcile.

From: regnets [mailto:notifications@github.com] Sent: Saturday, August 15, 2015 4:41 AM To: cyborg5/IRLib IRLib@noreply.github.com Cc: Chris Young cy_borg5@cyborg5.com Subject: Re: [IRLib] IRdecodeBase::decodeGeneric doesn't work with IR Commands where the data is in the spaces. (#12)

If you accept Pull Requests, i can make the changes. I'll just take the "sony" part out of the generic decode method, than i will change all the decodegeneric calls.

— Reply to this email directly or view it on GitHub https://github.com/cyborg5/IRLib/issues/12#issuecomment-131316299 . https://github.com/notifications/beacon/ADcguhHvuMQUWNs41CFUr68dpoHEBbKSks5onvKjgaJpZM4FrynT.gif

regnets commented 9 years ago

OK, thank you so far. I'll close this issue.