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

Parameter Conversion improvement #63

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

budulinek
Copy link
Contributor

  • Calculate bytestart during run-time
  • Add support for EHV
  • Bugfix 0x30 packet type

Calculate bytestart during runtime in order to avoid mistakes in the code. 
sizeValSeen is a sum of all elements in nr_bytes[][], still manually calculated.
Add support for EHV:
- Add support for 0x16 packet.
- Adjust nr_bytes for packet 0x400013
- Adjust nr_bytes for packet 0x400015
Bugfix: 
- Add nr_bytes for packet type 0x30
@Arnold-n
Copy link
Owner

Arnold-n commented Mar 4, 2023

Hi @budulinek,

Thanks for the EHV support extension, good to hear from you again, would love to add it!

As for the run-time bytestart calculation, I would rather not add it, as it results in a lot of avoidable CPU cycles. It avoids human mistakes in preparing the code, so I feel is it then better to calculate it once and store is in an array? Originally I considered that but didn't to reduce its (limited) RAM usage.

Arnold

@budulinek
Copy link
Contributor Author

Hi Arnold,
at first I was trying to figure out whether the bytestart calculation could be done during compile-time (in combination with macros). Here is a discussion suggesting that some calculations could be done during compile-time:
https://forum.arduino.cc/t/calculating-variables-at-compile-time/262409/18
https://forum.arduino.cc/t/run-time-vs-compile-time-functions/379649/8

If you are woried about CPU cycles, you can fill the bytestart[][] array during setup(). RAM usage? The code is supposed to run on ESP8266, so I think we can afford to spend few more bytes of RAM in order to avoid human mistake in the code. And by the way, why uint32_t bytestart and uint32_t nr_bytes?? uint16_t should suffice.

Yeah, I am working on a major upgrade of my Daikin P1P2 ⇔ UDP Gateway. I have noticed you have made a huge progress with P1P2 in the meantime, congratulations!

@Arnold-n
Copy link
Owner

Arnold-n commented Mar 4, 2023

Hi @budulinek ,
Thanks! I agree data size seems enough (80kB), but we were still running out of space, that is why the schedule code has been disabled for now. This code will disappear with the planned rewrite anyway. Indeed uint16_t for bytestart should suffice if in RAM, but it is easier to use uint32_t if in flash (PROGMEM).

Sorry, my mistake...
Works now.
@Arnold-n Arnold-n force-pushed the main branch 2 times, most recently from 4999f75 to e8ba40b Compare September 29, 2023 20:30
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