-
-
Notifications
You must be signed in to change notification settings - Fork 32
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
crash with interface connection but without internet #29
Comments
Config file was created to store some Hmm this internet crash looks like python libary crash, I hope they will fix it. |
if you did change The problem I see is that there are 2 different switches that are recorded into BUT, it just occurred to me that any config file should be in re: version #30, just change the version number, and have it automatically inserted into the string. Then you can explicitly match the version number when you do an update check. But like I mentioned, there are other TODO in the code as well. I dont mind providing a "good" (clean) PR of what I have done, with or without the seperated When do you think you might get around to the TODO lists in the code? I can probably do that too, but I dont want to make decisions for you if you already have any ideas. CommanderPi is a small (code) project, and not to complex (for me), so I have the time to get things working/changed/fixed pretty quickly (from my point of view - it still took 4 hours to make changes and create issue posts) |
Thanks for your commitment at this project. Honestly CommanderPI was my first big (big for me) project which I start coding while pandemic stuff and holiday started. Back to the topic, you are right that At now I don't have many ideas to improve CommanderPi, personally I was coding some stuff to controll fan with 3v3 and make depend it on |
I kinda thought that was the case. I will retract that PR and put something together in the next few days. Apart from the changes mentioned here, there are only aesthetic things (eg the network dialog, & change [ESC] text to button on all dialogs) - ie how things look (like the CPU output I fixed originally). Maybe get it to work on PC version of RaspberryPi OS (but I dont have a PC atm - could always use qemu I guess) back soon .. |
I did the new #37 |
Ok, I did some changes in this already but I don't know, is version in config is necessary. I don't think so that user can change version by config file so in my mind it should be still at I have re-writted comments in config as separate values started with ';' so now I don't see any bugs with writing/reading config and will drop |
I added
one reason to have yes, I was going to leave a
According to python docs, section does not have a value (INI style sections). I did think about changing I used One good reason not to keep EDIT: another reason to keep everything in the changes to one final note (I hope you tested the creating and rewriting properly, I only found the write problem by accident) - the way I constructed that read+create & update config sections, it will work if the filesystem is read-only (cant create) and/or the config file is read-only (cant update) EDIT: BTW if its specific to the |
(EDIT: I actually think this is a python bug, but the PR work-around avoids the crash)
I use a portal often, that means I have an interface connection, but until I login through the portal, I dont actually have an internet connection. The auto check fails at startup, way down deep inside a python library.
NOTE: this does not happen when I dont have an interface connection (eg if I disconnect wifi or ethernet).
I have patched the startup to accept a
auto_check = false
setting in thecpi.config
file as a work-around. I believe this to be a useful setting on some systems/situations. However, looking at the code, and theTODO
lists in the code, I would prefer to move all config stuff to a seperateconfig_cpi.py
and useimport config_cpi as cf
(if thats acceptable). I will submit a PR so you can see why I think it should be split off into a seperate file. (Also there is a issue withversion
in thecpi.config
)The text was updated successfully, but these errors were encountered: