search

jbuehl / solaredge

SolarEdge inverter logging data capture
GNU General Public License v3.0
288 stars 60 forks source link

Added bad key detection logic #118

Closed JohnOmernik closed 5 years ago

JohnOmernik commented 5 years ago

Added a section mostly from @jbuehl to detect a bad key. The one change was that if we have an active cipher (cipher is not none) and we detect the key is bad, to reset the cipher object upon detect.

JohnOmernik commented 5 years ago

So that's a good question. I think this is how I did it. I created se2file.py, you approved it. Then I updated the msg.py stuff did a pull request. Then before you approved the msg.py stuff, I updated se2file.py to add a directly based output (it doesn't change any default options, just lets you specify that files should go into directories by date) and pushed to my repo, now that is showing up in the pull request that I did. I am a git n00b. The changes to se2file.py should be fine if you just approve this one. It's tested, and working, and doesn't change anything in the off chance someone is already using se2file.py.

JohnOmernik commented 5 years ago

So, I realize now after some reading that if I were good githubbying citizen, I would have had two forks for my two individual changes. I would have pushed the bad key detection off a different fork, and then done the se2file.py update in a different pull. Still trying to learn how to contribute logically.

Two paths:

  1. Reject the change and force me to do it correctly.

  2. Accept the change, look at the bad key detection and trust that the change to se2file.py is ok.

  3. Would be easier for me. And I promise not to do it again. The changes to se2file that would come along with the pull request will essentially just add functionality, and it has been tested. Both are tested and are working. If you just accept this pull, I will clean up my end, sync up, and ensure any changes that I am I working on in parallel are in separate forks. Sorry for the confusion.

Once this is cleaned up, I will sync, and focus on the key rewrite stuff. It's already working and useful in my dev setup, but I want to clean the code, and make it acceptable to be in main.

jbuehl commented 5 years ago

OK, hopefully I didn't break it.

JohnOmernik commented 5 years ago

Sigh, no I did I think... let me fix.

JohnOmernik commented 5 years ago

Ok, my bad. Look at #120 and that will fix an indent problem I didn't check for correctly. That's on me, if you merge #120, we should be good to go. Sorry, I am just not good at git yet. (But I do know that you can always undo stuff, or so I've been told and have found to be true)