flipperzero-firmware
flipperzero-firmware copied to clipboard
flipperdevices
Infrared: Fix not enough repeats for parsed protocols in brute mode
What's new
- Fixes SIRC protocol now works fine with sony tvs in universal remote
- May fix other
parsedprotocols that was sent and not received due to too short signal
Verification
- Remove RAW recordings from infrared/assets/tv.ir file (some of them are just not parsed SIRC)
- Make sure to have parsed SIRC buttons for Sony Bravia TV in file
- Verify that bruteforce now actually works (we tested it on 2 Sony tv's with SIRC protocol)
Checklist (For Reviewer)
- [ ] PR has description of feature/bug or link to Confluence/Jira task
- [ ] Description contains actions to verify feature/bugfix
- [ ] I've built this code, uploaded it to the device and verified feature/bugfix
I have tested this and verified it works, tried on official dev, removed raw data and Sony tvs will no longer work, the current button that works for Sony Bravia is power only and this is because the standard parsed power button is also in a raw data format.
We have also tested getting this parsed code (P: SIRC, A: 01 00 00 00 , C: 15 00 00 00) and receiving it in raw using ir rx raw and replaying it via the universal remote and the signal worked because the signal was long enough to be received by the device.
We have tested and verified the fix, the buttons do not spam under universal remote (eg use vol+ and tv recognised 3 vol+ presses) and the remote operates as it should. We have also had multiple Bravia users test this and it has the intended and correct behaviour, no multiple buttons presses received by the device and the original parsed codes are recognised.
If you have a Bravia TV I'd suggest removing all raw data from your tv.ir and trying for yourself and you will immediately see the issue and also see the fix resolves this under the universal remote.
The reason this was done with all parsed codes was the simply because there was a lot of users reporting a lot of common brands as not working. So it gave us the indication that it is likely not just the SIRC protocol that's having this issue. Though it seemed the most mentioned.
As to why it's happening with SIRC (and likely other parsed codes) your guess is as good as mine, being we captured the normal parsed code (A: 01 00 00 00, C: 15 00 00 00) and turned this into raw data and it worked my best guess is not a long enough signal to be picked up by the device because of how fast it is sending.
I hope this helps and provides a little more context!
I don't think that we can just send the signal 3 times willy-nilly, as it breaks the expected behaviour of the program (user presses the button once and expects one signal to be sent).
What does a real remote for that TV do? Does it send a signal several times, too? If not, how does it make sure that it gets recognised by the TV?
More so, sending all parsed signals three times will needlessly increase the brute force run time.
I think (assuming repeating signals is the way to go) that we'll need some heuristics which will decide how many times a signal should be sent (and it should affect only the universal remotes, as the user may send the signal manually as many times as they like).
The simplest method would be to repeat (again, assuming that repeating is the right way to do it) only the SIRC messages (what is it about them? are they all inherently short? I've no idea...).
@amec0e provided all details about this case What I can add to this, yes it can be modified to add repeats only for SIRC, because signal is too short while transmitting it with bruteforce, in regular remote signal transmits while you holding a button, so this doesn’t affect regular remotes this function is used only in brute and one cli command
If requested change requires only adding a check for SIRC protocol, tell me I will do changes to this PR
Repeating payload couple times do make sense. I've seen some receiver implementation that explicitly waits for payload to repeat before recognizing it.
At the same time I understand @gsurkov concerns: we want to make sure that there are not issues in encoder/decoder itself before repeating payload.
We have some sony remotes in the office, we'll check how they behaves and then decide if this is enough or more work needed.
Repeating payload couple times do make sense. I've seen some receiver implementation that explicitly waits for payload to repeat before recognizing it.
At the same time I understand @gsurkov concerns: we want to make sure that there are not issues in encoder/decoder itself before repeating payload.
We have some sony remotes in the office, we'll check how they behaves and then decide if this is enough or more work needed.
We 100% understand the concern, in fact most of @gsurkov question's asked we had asked ourselves as of course we don't want to hinder the performance of the universal remote any more than is needed but while still retaining functioning codes.
By all means please do take a look on the encoder/decoder side of things to make sure everything there is fine and that repeating the payload is the best way to go. We tested repeating the payload twice and that still works with Sony Bravia devices.
I have done some preliminary research and can confirm that for some remotes it is normal to send a signal several times in a row, even if the user does not hold the button.
For example, this commentary in the code states that Sony remotes always send the signal at least 3 times, and there is no special form for repeated message. (Why didn't I know this before? Because I have inherited this code! :) )
This led me to believe that the number of repeats should be a protocol-defined variable (and not hardcoded for all protocols). I will check all of the available remotes ASAP to see if some of them also have a minimum number of repeats. In this light, just increasing the number of repeats everywhere is starting to look like an acceptable fallback plan (if there will be no definitive answer), although there is no guarantee in this case that some devices won't react unexpectedly.
Glad to see this being discussed. I worked with xMasterX and Amec0e for testing and such on this. I've personally confirmed this fix was the fix for my non-working Sony Bravia TV. In fact, it runs pretty quickly in comparison to Official as it removes the RAW which takes longer to send than the SIRC.
@xMasterX @amec0e please try #2293 to see if it fixes your problem.
Long story short, it enables each protocol to have a minimum number of repeats when transmitting. For now, only SIRC has this number other than 1 (3, to be exact), but it can be easily changed if needed. This number applies to both brute force and worker modes.
There also was some refactoring of the protocol definitions, but that should not change anything functionality-wise.
Additionally, some raw messages in the tv.ir library were converted to messages (where possible). Only one SIRC signal was detected, by the way.
I have done some preliminary research and can confirm that for some remotes it is normal to send a signal several times in a row, even if the user does not hold the button.
For example, this commentary in the code states that Sony remotes always send the signal at least 3 times, and there is no special form for repeated message. (Why didn't I know this before? Because I have inherited this code! :) )
This led me to believe that the number of repeats should be a protocol-defined variable (and not hardcoded for all protocols). I will check all of the available remotes ASAP to see if some of them also have a minimum number of repeats. In this light, just increasing the number of repeats everywhere is starting to look like an acceptable fallback plan (if there will be no definitive answer), although there is no guarantee in this case that some devices won't react unexpectedly.
Sorry to get back to you so late, that is awesome, I am very curious to know which devices/remotes send signals several times if possible, some of this is news to me.
Most definitely I do agree it should likely be a protocol defined adjustment as to only affect SIRC (for now) as this is where the issue is showing most prominently.
In your testing did you find the repeats/fix sent causing any unexpected behaviour?
@xMasterX @amec0e please try #2293 to see if it fixes your problem.
Long story short, it enables each protocol to have a minimum number of repeats when transmitting. For now, only SIRC has this number other than 1 (3, to be exact), but it can be easily changed if needed. This number applies to both brute force and worker modes.
There also was some refactoring of the protocol definitions, but that should not change anything functionality-wise.
Additionally, some raw messages in the
tv.irlibrary were converted to messages (where possible). Only one SIRC signal was detected, by the way.
For sure I can give this a test in a little bit and get back to you on it hopefully by the end of the day (some testers are in different time zones)
As for the conversion of raw messages yes I believe there was only the one Sony code in raw data among your tv.ir file which was POWER if I'm not mistaken. Which you should not need to be in there if your parsed codes for Sony are now working.
I am super curious on how you converted the raw data into messages (which I'm guessing is parsed data if I'm not mistaken?) would appreciate knowing how to do that :D
which devices/remotes send signals several times
I have recorded such behaviour only in the Sony remote, which is consistent with the protocol description. In other remotes, I was able to produce only one message upon a very short button press. Note: Most (if not all) remotes do repeat the signals if a button is held (just so we' re clear).
It was not a very thorough or controlled test, though, and the set of remotes was limited by what we had on hands.
In your testing did you find the repeats/fix sent causing any unexpected behaviour?
There was not much to test this on, hence only SIRC gets 3 repeats for now, as there is evidence for that in the protocol specs and the real hardware.
I do not expect regular TVs and other such appliances to react weirdly, however, as the remotes usually repeat signals rather frequently, so it's hard for the user not to send a bunch of messages at once. Therefore, the receiving end must have some sort of timeout built into it. It would be best not to rely on this at all.
I am super curious on how you converted the raw data into messages (which I'm guessing is parsed data if I'm not mistaken?) would appreciate knowing how to do that :D
There is a decode option in the ir console command, which can parse known raw signals and replace them with decoded messages either in the original file or in a copy. It employs the very same decoders used in the regular signal analysis.
I have recorded such behaviour only in the Sony remote, which is consistent with the protocol description. In other remotes, I was able to produce only one message upon a very short button press. Note: Most (if not all) remotes do repeat the signals if a button is held (just so we' re clear).
Okay that's absolutely awesome, I was able to have your fix confirmed working on 2 different Sony Bravia models, just waiting on our third Sony Bravia tester to ensure this is still indeed all good. Also the "Note:" is handy to know I appreciate you taking the time for me to help clarify some of these questions.
It was not a very thorough or controlled test, though, and the set of remotes was limited by what we had on hands.
There was not much to test this on, hence only SIRC gets 3 repeats for now, as there is evidence for that in the protocol specs and the real hardware.
This is absolutely fine between a handful of us we have a lot of devices to test on with the main culprit being Sony Bravia.
I do not expect regular TVs and other such appliances to react weirdly, however, as the remotes usually repeat signals rather frequently, so it's hard for the user not to send a bunch of messages at once. Therefore, the receiving end must have some sort of timeout built into it. It would be best not to rely on this at all.
Okay that's awesome, I did test on a few non Sony devices myself and all did seem fine. Again I appreciate the info!
There is a
decodeoption in theirconsole command, which can parse known raw signals and replace them with decoded messages either in the original file or in a copy. It employs the very same decoders used in the regular signal analysis.
Super, I did actually find this a few hours later and tried it on some raw data, very useful to know thank you!