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

[IRTX] Use parse argument functions for IR TX command arguments #3404

Merged
merged 6 commits into from
Dec 4, 2020

Conversation

TD-er
Copy link
Member

@TD-er TD-er commented Nov 30, 2020

As mentioned here: #2724 (comment)

@TD-er
Copy link
Member Author

TD-er commented Nov 30, 2020


StaticJsonDocument<200> docTemp;
DeserializationError error = deserializeJson(docTemp, cmd.substring(cmd.indexOf(',') + 1, cmd.length()));
DeserializationError error = deserializeJson(docTemp, parseStringToEnd(cmd, 2));
Copy link
Contributor

Choose a reason for hiding this comment

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

cmd.length() is missing.

Copy link
Member Author

Choose a reason for hiding this comment

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

But that's only needed for substring, right?
parseStringToEnd is doing all the magic for you, including stripping matching quotes if present.
It does also convert to lower case.

@jimmys01
Copy link
Contributor

@jimmys01

Can you try this test build?

* [ESPEasy_ESP32_mega-20201130-3-PR_3403.zip](https://www.dropbox.com/s/rgk4qda90x070xn/ESPEasy_ESP32_mega-20201130-3-PR_3403.zip?dl=0)

* [ESPEasy_ESP82xx_mega-20201130-3-PR_3403.zip](https://www.dropbox.com/s/dayp8wexthknyva/ESPEasy_ESP82xx_mega-20201130-3-PR_3403.zip?dl=0)

give me 30 min

@jimmys01
Copy link
Contributor

Same behavior.
At least for the IRSENDAC command

@jimmys01
Copy link
Contributor

jimmys01 commented Nov 30, 2020

#ifdef P016_P035_USE_RAW_RAW2
          handleRawRaw2Encoding(command);
#endif //P016_P035_USE_RAW_RAW2
          handleIRremote(command);
        }

Also lines 145 to 149 need your special treatment?
I am to sleep deprived right now, I will have a better look tomorow

@TD-er
Copy link
Member Author

TD-er commented Nov 30, 2020

Added the changes to those functions too.

@jimmys01
Copy link
Contributor

jimmys01 commented Dec 1, 2020

Works, thank you.

Also the custom_ir build seems broken once more.

xtensa-lx106-elf-g++: error: CreateProcess: No such file or directory
*** [.pio\build\custom_IR_ESP8266_4M1M\ESP_Easy_mega_20201201_custom_IR_ESP8266_4M1M.elf] Error 1

@TD-er
Copy link
Member Author

TD-er commented Dec 1, 2020

Yep I noticed.
We're constantly hitting the max. command line limit in Windows.
I will poke the platformIO developers onece more to stress the importance of a work-around on that issue.

@TD-er
Copy link
Member Author

TD-er commented Dec 2, 2020

@jimmys01 I managed to make a nice work-around for the longcmd bug.
See: platformio/platform-espressif8266#231

Just performing a few last tests and if those don't show serious issues, I will merge my PR for this so you can build it all again on Windows.

@TD-er
Copy link
Member Author

TD-er commented Dec 3, 2020

@jimmys01 I merged the PR so please let me know if you run into build issues again.

Does this PR need anything else or is it now working?

@jimmys01
Copy link
Contributor

jimmys01 commented Dec 3, 2020

just to be sure the, command
IRSEND,RAW2,something,38,something
must be transformed to
IRSEND,'RAW2','something','38','something' ?

@TD-er
Copy link
Member Author

TD-er commented Dec 3, 2020

quotes are only needed if the parameter contains a character that can be considered a parameter separator, like:

  • , (a comma)
  • (a space)

And to make it possible to still use single or double quotes in a parameter, you can chose either of the other quote types to wrap a parameter.

So if you only have parameters that don't have a comma or a space, then you don't need to add quotes.

@jimmys01
Copy link
Contributor

jimmys01 commented Dec 3, 2020

We are good to go! also the Custom IR builds work again!! Extremely good work, it seemed a really tricky bug to solve.
One minor issue is that if you press submit in the following screen

εικόνα

the updated page will be missing the parameters and only hold the first part of the command.

εικόνα

just reporting it because i noticed it, nothing really bad comes out of it.

@TD-er
Copy link
Member Author

TD-er commented Dec 3, 2020

@jimmys01
If you ever have "just reporting" feeling itching... Please be aware what it may result into ;)
See the last commit I added to this PR.

@TD-er TD-er merged commit e8c7781 into letscontrolit:mega Dec 4, 2020
@TD-er TD-er deleted the bugfix/Parse_IR_commands branch December 4, 2020 11:17
CurlyMoo pushed a commit to CurlyMoo/ESPEasy that referenced this pull request Dec 4, 2020
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.

2 participants