Closed fwolfst closed 1 year ago
@fwolfst sadly it is not that simple. The protocol remains the same of course but we cannot use the same code for the esp32. String operations like strncpy and srtncat make the ESP32 crash. I had a lot of troubles using the sLen and crc functions for instance, it just didn't work and crashed the SP32. As for the RPI, we are talking not only about C++ but also about Perl, Java, PHP, Bash and maybe i forgot one. So this is different. And it is working fine so i rather keep my hands off it.
The reason i merged your suggestions manually is that i want to test them in my own environment. At the end i want to understand the code myself, you see. By pushing a button i can't learn anything. As to your other remarks, for now i will remove your name. Just wanted to be polite but you are right.
@patience4711 thanks for the update.
[...] we cannot use the same code for the esp32. String operations like strncpy and srtncat make the ESP32 crash. I had a lot of troubles using the sLen and crc functions for instance, it just didn't work and crashed the SP32.
It might crash because of untypical use of char *
. Some of the code failed to execute in native linux runtimes (from my limited understanding: because of return
ing local char - which are freed after being returned). Maybe rpi and esp32 compilers do the same? In that case the code would have to be rewritten to accept a `char ` as destination instead of returning it.
I understood the case of manual changes (although there would be a way to do this with git).
As to your other remarks, for now i will remove your name. Just wanted to be polite but you are right.
:+1:
@fwolfst my knowledge is very limited, If the compiler doesn't complain but the execution crashes, memory violation is the most obvious cause. Could be the use of those pointers, as a workaround, in the ESP32 i return a string and that works but is not ideal. The Raspberry isn't so picky about the code, it just does its work.
I am experimenting with some hopefully memory safe refactoring: https://github.com/fwolfst/ECU-ESP8266/blob/main/src/zig.cpp. It will not compile yet, but maybe it can become helpful one day. Creating Pull Requests and keeping a fork in sync with the code here was a bit too difficult (e.g. because compiler on linux behaves different and I always have to rename the files etc.pp.).
@patience4711 If you want we can exchange wrt to how the memory stuff works and why some things might be problematic. But I am also still learning and not really sure I do the right things.
@fwolfst I have a version running with a function sendZigbee that adds sLen and CrC , and a function char * readZigbee that returns the incoming message. Now the global "inMessage" is not needed anymore. There is a memory gain but i can't say that this really matters. All and all a lot of trial and error work. I am currently running a test with one YC600 and it works like a charm so the engine seems oke. Ofcourse it doesn't work in the ESP32. So i must confess that i am losing my interest in this stuff, i want to get my life back!
I just examened your library work, i must say i am impressed, you seem the be a lot more a programmer than i am. It looks very professional. I really hope that you succeed in making this work. For now i am taking a break with this, i have work enough with the issues (although there seem none at the present.) Maybe in future i find the time to dive into your code and hope i can learn of it.
@patience4711 I just looked at your changes, looks good! I am a professional programmer, but not with c/c++ . Yes it takes a lot of time, and if you lost your life, get it back! Your tool works and has really a lot to offer already. Good job. Enjoy your break and feel free to reach out.
@fwolfst One of the differences between ESP8622 and ESP32 is this statement that works in the ESP8266 but fails in ESP32 strcpy(printString, strcat(sLen(printString), printString)); The strcat part loads the stack and in the ESP32 this is considered a violation. The stack seems better garded. Maybe usefull for you to know. So i made a solution that works on both platforms and makes the function sLen redundant.
Nice catch!
Hi @fwolfst
I have simplyfied some processes , the pairing, starting the coordinator and sending / reading the zigbee messages. Spinn-off of the esp32 project.
It is working nicely but i wonder if it is memory efficient. I surely gained some program space. In the past the program was built to spit out debug info as much as possible to be able to see what was happening. Now we can skip a lot of that. Anyway, i have more confidence in your approach as to memory efficiency so if you have some spare time, maybe you see some improvement opportunities.
In particular, the readZB function that returns a char, this seems a questionable technique. This line "strcpy(messageToDecode, readZB(s_d));" seems impossible because it seems to convert the char to a char. Still it works..
Btw i was once in Osnabruck, when the Rampendahler was recently launched. Long time ago....
Hi @fwolfst I have simplyfied some processes , the pairing, starting the coordinator and sending / reading the zigbee messages. Spinn-off of the esp32 project.
I briefly looked over the changes, looks good!
It is working nicely but i wonder if it is memory efficient. I surely gained some program space. In the past the program was built to spit out debug info as much as possible to be able to see what was happening. Now we can skip a lot of that. Anyway, i have more confidence in your approach as to memory efficiency so if you have some spare time, maybe you see some improvement opportunities.
If I get my code working I can make a suggestion, will focus on that and unfortunately get pretty limited time.
In particular, the readZB function that returns a char, this seems a questionable technique. This line "strcpy(messageToDecode, readZB(s_d));" seems impossible because it seems to convert the char to a char. Still it works..
I don't find that code and having a global shared inputbuffer might have been be a good idea. I am not yet sure how I will deal with it. Generally speaking you should never return non-global char * !
Btw i was once in Osnabruck, when the Rampendahler was recently launched. Long time ago....
Havent been there for years unfortunately. Out of curiosity:, where did you find the Osnabrück reference about me?
@fwolfst I clicked some things and I believe i saw that on linkedin, university of Osnabruck. But maybe that wasn't you, the profile could fit though.
I saw that there are all these other cool projects to run the app on different hardware platforms (rpi, esp32). They most likely share the same code for talking to the inverters via zigbee. So this might make a good refactoring/extraction target. Whether in a full fledged libary to include into Arduino IDE/platform io, or files that can be copied over. I was minimally playing around to try to do so, but understand too few of the protocol as of yet. @patience4711 What do you think?
Btw i have seen that my pull requests are not merged but maybe manually copied over - is there some support needed, or is the workflow fine (if there are questions about the tooling, I am happy to show around how many/most developers use Pull Request + git/Github).
Also, thanks for mentioning me in the code and/or readme. While this is nice for bigger changes, for these small changes its not needed from my point of view (and it would even less, if ownership is carried in the git history). In the code file it is actually probably more annoying for new readers (they have to scroll over it, since its in the beginning of the file). I appreciate it, but feel free to condense it to one line in the README ("thanks to everyone who contributed: a, b, c"), or skip my name alltogether.