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

add datetime to stdout and date to swjsq.log #82

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fzinfz
Copy link

@fzinfz fzinfz commented Dec 14, 2016

add datetime to console output and swjsq.log for easier debugging script failures.

@timothyqiu
Copy link
Contributor

IMHO, it'll be better to use logging instead of writing files manually.

fix TypeError
@fzinfz
Copy link
Author

fzinfz commented Dec 14, 2016

I agree with @timothyqiu that logging module is better. Since it requires large modifications, it would be better let @fffonion decide if it's necessary.
In this pull request I just did few safer modifications based on the original code to record timestamp for easier debugging.

@fffonion
Copy link
Owner

I'm not sure if logging is included in the python install of openwrt. We can use logging when available and use plain print as fallback. Also it would be better to wrap this into a function and replace every print with our new function.
For example:

try:
    import logging
    def _log(s):
        logging.log(...)
except ImportError:
    def _log(s):
        print("%s: %s" % (strftime, s))

And please make sure to consider unicode and python3.
Thanks a lot : )

@fzinfz
Copy link
Author

fzinfz commented Apr 15, 2017

Sorry for late reply. After this post, I migrated this tool from my MPIS32 Debian router to windows Anaconda3 environment running with powershell. It's been running very stable since then, and no login failed issues any more. Since my broadband will be upgraded to 100M directly, I won't use this tool often and won't make further modification probably. Thanks for providing such great tool!

@fffonion
Copy link
Owner

@fzinfz Thanks for you contribution anyway !

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.

3 participants