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

Chuangmi IR: Add ability to send any Pronto Hex encoded IR command. #183

Merged
merged 1 commit into from
Jan 27, 2018

Conversation

yawor
Copy link
Contributor

@yawor yawor commented Jan 26, 2018

Pronto Hex is probably the most popular format for storing IR signals.

A usage example:

from miio.chuangmi_ir import ChuangmiIr
c = ChuangmiIr(dev_ip, dev_token)
c.play_pronto('0000 006C 0022 0002 015B 00AD 0016 0016 0016 0016 0016 0016 0016 0016 0016 0016 0016 0016 0016 0016 0016 0016 0016 0041 0016 0041 0016 0041 0016 0041 0016 0041 0016 0041 0016 0041 0016 0016 0016 0016 0016 0041 0016 0016 0016 0041 0016 0016 0016 0016 0016 0016 0016 0016 0016 0041 0016 0016 0016 0041 0016 0016 0016 0041 0016 0041 0016 0041 0016 0041 0016 0623 015B 0057 0016 0E6E', repeats=2)

It only supports a raw variant of Pronto, which begins with value "0000".

repeats += 1

times = set()
for pair in pronto_data.intro + pronto_data.repeat * (1 if repeats else 0):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (83 > 79 characters)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 62.496% when pulling 9d58a49 on yawor:chuangmi_ir_pronto into e9ce9e0 on rytilahti:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 62.496% when pulling 9d58a49 on yawor:chuangmi_ir_pronto into e9ce9e0 on rytilahti:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 62.496% when pulling 9d58a49 on yawor:chuangmi_ir_pronto into e9ce9e0 on rytilahti:master.

@cnrd
Copy link

cnrd commented Jan 26, 2018

Any way that we could make this part of the .play command and detect whether the command is base64 (captured) or Proton format?

@yawor
Copy link
Contributor Author

yawor commented Jan 26, 2018

@cnrd I don't think that's a good idea. It adds much more complexity. Also you don't need to put spaces in the Pronto (not Proton :)) and it would make hard to recognise if this is Pronto or base64 (Pronto doesn't use any characters which can't be used in base64). In case of Pronto, the frequency is already in the signal data and my method also gives you an ability to define how many repeats you want (Pronto has separate sections for intro sequence and repeat sequence).

Do you see any use case of play method directly accepting a Pronto code?

@cnrd
Copy link

cnrd commented Jan 26, 2018

Pronto noted. The use case that I would have is in the home assistant platform I'm currently writing, this would avoid requiring extra parameters to be set for each command. Imagine a list of commands, if we could detect the signal type then we could make a list like this:

  • base64
  • base64
  • pronto

If we cannot detect the type we would need this to be:

  • command:
    • base64
    • command_type: base64
  • command:
    • base64
    • command_type: base64
  • command:
    • pronto
    • command_type: pronto

I do however agree that if we cannot detect it reliably then the second option is better even if it's a bit more verbose.

Also about the repeat section (I know close to nothing about IR signals): Is this a parameter that repeats the signal to make sure is is received correctly, or is it something that is required by some devices, as in something that would usually be a part of a pronto command found in a manual or on the internet?

Sorry for the formatting I'm currently on mobile.

@yawor
Copy link
Contributor Author

yawor commented Jan 27, 2018

If you write commands as a simple list, then how do you plan to provide a frequency when base64 is used? You'd still need to make a dict with with code and frequency. And for Pronto you'd want to be able to provide number of repeats.

I wonder if the autodetection should be on the library side or on the HA component side. You can have much more control if you do this in the component and then just call the right method in the library.

The Pronto code in the example I've provided in the comment:

0000 006C 0022 0002 015B 00AD 0016 0016 0016 0016 0016 0016 0016 0016 0016 0016 0016 0016 0016 0016 0016 0016 0016 0041 0016 0041 0016 0041 0016 0041 0016 0041 0016 0041 0016 0041 0016 0016 0016 0016 0016 0041 0016 0016 0016 0041 0016 0016 0016 0016 0016 0016 0016 0016 0016 0041 0016 0016 0016 0041 0016 0016 0016 0041 0016 0041 0016 0041 0016 0041 0016 0623 015B 0057 0016 0E6E

is the one I've used to generate the base64 version you've tested successfully in #175. You can either use it in a format like above for readability or you can remove the spaces. If you remove the spaces then the character set used in the code is the subset of charset used in base64. Of course there's really low chance that this would be an actual base64 string, especially because the binary form of the signal starts with the magic sequence 0x67, 0xa5, which should always produce a base64 string starting with "Z6".
So the simplest and most naive method of autodetection would be to check if the string starts either with "Z6" or "0000" and report an error if anything else is provided.

Instead of autodetection you could also go with something like this: raw:BASE64_STRING[:OPTIONAL_FREQUENCY] and pronto:PRONTO_STRING[:OPTIONAL_REPEAT_COUNT] and parse it in the component and then pass to correct method. The colon character is not allowed in neither base64 nor pronto so such format would be safe to use and future proof in case the library would add support for another IR format.

@syssi syssi merged commit 2eaa154 into rytilahti:master Jan 27, 2018
@rytilahti
Copy link
Owner

Just a small remark about autodetecting, maybe it makes sense to have one as a default (accompanied with debug logging for those tinkerers among us), and let one to cast with a syntax as stated in @yawor's last comment (<type>:<data>:<freq>) where needed?

I would not necessarily find using a simple heuristics to try to detect the type (e.g. a base64-encoded value may = characters in the end to indicate padding, character distribution seems to be different too from the quick look) that bad either, but this would require creating a test-suite for the detector with enough samples from both types. On the other hand whether this is worth the time spent, I doubt it..

@yawor
Copy link
Contributor Author

yawor commented Jan 27, 2018

@rytilahti maybe it could be done with another method in addition the the play and play_pronto. Or maybe the play could be renamed to play_raw and new play could be introduced with the semantics I've mentioned in my comment which would in turn use the play_* methods as backends. That way the software could either use the built-in routing or use the methods explicitly.

@yawor yawor deleted the chuangmi_ir_pronto branch January 28, 2018 00:07
@rytilahti
Copy link
Owner

@yawor I think that would be the best way to handle this indeed, I would really not prefer to have something like in users' code: https://github.com/home-assistant/home-assistant/pull/11891/files#diff-c3a98a9bc5ffb47c62c48b7f7b114de3R231 .

So to summarize what we'd need to change:

  • Rename current play to play_raw
  • Add a check what type of payload is given and forward to those accordingly.
  • When no type is explicitly given, fall back to raw (considering it's what the device natively speaks and returns with ir_read)?

What do you think?

@syssi
Copy link
Collaborator

syssi commented Jan 28, 2018

@rytilahti Do you prefer the "Z6" vs. "0000" type check?

@yawor
Copy link
Contributor Author

yawor commented Jan 28, 2018

@rytilahti so it could work like this:

  1. Split the incoming parameter on colon.
  2. Check resulting length:
    • if 1, then we assume that it contains only data, try autodetection and call appropriate method with sane defaults for optional parameters
    • if 2, then we assume that it is <type>:<data> and call appropriate method explicitly with sane defaults for optional parameters
    • if 3 or more (future proof), we assume it's <type>:<data>:<param1>[:<param2>[...]], validate and process extra parameters according to the specified type and proceed with explicit call to appropriate method

Is that correct?

@rytilahti
Copy link
Owner

@syssi I think both of them are fine, shouldn't really matter (except for detecting an unknown formats).

@yawor that sounds good. I think the first part should be to check if there's a colon. If yes, read the type and pass the rest of it to the appropriate handler (that is, the type part is not delivered forwards).

@airdropbelgium airdropbelgium mentioned this pull request Sep 25, 2018
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.

6 participants