patience4711 / read-APSystems-YC600-QS1-DS3

Software for an esp8266 nodemcu to read data from APS inverters.
127 stars 24 forks source link

License unclear #105

Closed fwolfst closed 1 year ago

fwolfst commented 1 year ago

I am very grateful for all the work put into this by everyone involved. Now that I have my ECU running, I would like to play a bit around with the code. I am thinking about feeding the data into the LoRaWAN and suspect that there are some string operations that can be optimized, etc.

Unfortunately, the license situation is unclear, such that I don't know how/if I can legally modify the code in public (e.g. a "fork"). That might sound a bit childish, but since I wish for a world where sharing and freedom is taken seriously, I would like to do so explicitely.

I know a good lot about licensing and if you want we can have a phone call or chat about this topic. If you choose to make the licensing and copyright situation clear (and have to decide on a license) - personally I am more on the Free and Open Source side and would choose the AGPLv3+ - but there are many other good alternatives, too (including staying proprietary as it is now). There are also tools that try to help you find the right fit (e.g. https://choosealicense.com/) .

patience4711 commented 1 year ago

If you publish your source code in a public repository on GitHub, you have accepted the Terms of Service, by which you allow others to view and fork your repository.

This seems clear enough to me. I don't know what your goal is but I know that there are many people who prefer their own logo in the webinterface. Or a translation in their own language. Be my guest. But if you want to publish your version under your name, i wouldn't be amused of course. Some things you just don't do, anyway i wouldn't.

I don't like this either: "I suspect that there are some string operations that can be optimized", to me this sounds a bit arrogant. But you could be right about that.

If you feel that you can or must contribute, and want to be nice, pull requests are your best friend. And please tell me how i can optimize the stringoperations.

fwolfst commented 1 year ago

If you publish your source code in a public repository on GitHub, you have accepted the Terms of Service, by which you allow others to view and fork your repository.

This seems clear enough to me. I don't know what your goal is but I know that there are many people who prefer their own logo in the webinterface. Or a translation in their own language. Be my guest. But if you want to publish your version under your name, i wouldn't be amused of course. Some things you just don't do, anyway i wouldn't.

Maybe you should write something along these lines in the README so that people know that. The situation without a given license or copyright is unclear to most people.

I don't like this either: "I suspect that there are some string operations that can be optimized", to me this sounds a bit arrogant. But you could be right about that.

I am sorry if it sounded arrogant. Neither of us are hardcore C/++ programmers from what I can see. I am (re-)learning the stuff and stumbled about this: https://github.com/patience4711/read-APSystems-YC600-QS1-DS3/commit/6e075a0b7401c78f9c9dcb882e2d952b4e2e0f62 . I think both the CRC and the Len computation and string concatenations (<LEN>command<CRC>) could be moved into sendZigbee(command) - I will play with that and happily craft a PR when I find time during sun-hours (i have no setup to test independent from my inverter).

If you feel that you can or must contribute, and want to be nice, pull requests are your best friend. And please tell me how i can optimize the stringoperations.

I feel a bit misunderstood - my issue was more about permission to contribute to the overall idea (be it in a public fork or into this repository) than about "doing my thing". I contributed to dozens of Open Source projects and will happily create a PR if I find something that can be improved (e.g. reponsabilities of the sendZigbee() method). Btw in the fork I also made some modifications to make compiler warnings disappear (e.g. missing returns). Next step for me would be to let the formatter run over all files for consistent indentation.

Before we run into misunderstandings, I am always happy to network and thank you "in person" in chat, phone or videocall - please reach out if you feel misunderstood or have a bitter sentiment left. I asked here with the best intentions and are grateful for you sharing this projects and even providing some real support in the github issues.

patience4711 commented 1 year ago

You really take this seriously, don't you? Why? You should know that there is no money in it and only a little honor. Most people don't even bother to say thanks. But oke. i am convinced of your noble intentions. Saw that you already found a lot of imperfections. It's good to have someone take a critical look at it. I especially like that making the uppercase function obsolete.

I already had a new way te append the crc. I now do this within the zendZigbee function so all other crc operations are obsolete and removed. We could do that too for the len but in the polling command this is hardcoded and the other commands are not used very often so why bother.

Thanks for your input and keep up the good work...

joffito commented 1 year ago

It's cool to have some activity on this project. Sure there is no money or "fame" involved but the open source thought can be lived. I am by no means a good coder but i'd also have some ideas how to streamline the code: no use of serial in the compiled version (if you do), make variables as small as possible (byte instead of int e.g.), low level register writes (instead of digitalWrite or attachInterrupt) etc. pp. As patience4711 said: why bother ? It's already quite good. And the latestet release (which i still have to flash) is probably very stable. But of course, if the code gets more and more "conform" to standard it's a plus for everybody. Thansk again for everybody bringing this forward. I'll buy the next drink ;-)