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

Logging improvements #260

Merged
merged 1 commit into from
Jan 4, 2025
Merged

Logging improvements #260

merged 1 commit into from
Jan 4, 2025

Conversation

Ape
Copy link
Contributor

@Ape Ape commented Jan 4, 2025

The first commit disables a huge number of completely useless logging messages about the logging library itself.

The second commit is a bit opionated, but I really don't want this application to log to a file by default. At the very least the log path should be something more appropriate (e.g. $XDG_DATA_HOME).

@sblantipodi
Copy link
Owner

sblantipodi commented Jan 4, 2025

Hi @Ape,
thanks again for your contribution, it's really appreciated.

I agree that Luciferin defaults to a log level that is quite verbose but disabling logging entirely is not a good option in my opinion.
All apps must have a log for debugging purposes.

I'll change the default log level to ERROR
and I'll move the log file to the $XDG_DATA_HOME in the next release (or the next after the next).

In this way no log file will be created if there are no errors, and the log file will be in the $XDG_DATA_HOME folder.

I leave this PR opened just to remember that it needs additional work.

PS: As I said the the feature request I already have some methods that can move the files to another path without breaking existing installation, so I can continue it :)

Copy link
Owner

@sblantipodi sblantipodi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as discussed in the PR, can you remove the second commit please?
log must not be entirely disabled, I will disable it by default.

@Ape
Copy link
Contributor Author

Ape commented Jan 4, 2025

I agree that Luciferin defaults to a log level that is quite verbose but disabling logging entirely is not a good option in my opinion. All apps must have a log for debugging purposes.

I'll change the default log level to ERROR and I'll move the log file to the $XDG_DATA_HOME in the next release (or the next after the next).

I think the current log level is good. I just disagree with the log location.

The current logging to stdout is good and I'd definitely leave it at INFO level.

If you move the log file to $XDG_DATA_HOME, then maybe it's better to leave that at INFO level, too.

Anyway, I will remove the second commit here, so we can get the first one merged.

@Ape
Copy link
Contributor Author

Ape commented Jan 4, 2025

Also, note that $XDG_DATA_HOME is usually and should default to ~/.local/share and is different from $XDG_CONFIG_HOME.

@sblantipodi
Copy link
Owner

sblantipodi commented Jan 4, 2025

Also, note that $XDG_DATA_HOME is usually and should default to ~/.local/share and is different from $XDG_CONFIG_HOME.

yes I used XDG_CONFIG_HOME :)
thank you Ape, merging it and releasing it on the next release.

@sblantipodi sblantipodi merged commit 2f8571e into sblantipodi:master Jan 4, 2025
2 checks passed
@sblantipodi sblantipodi mentioned this pull request Jan 4, 2025
@Ape Ape deleted the pr/logging branch January 4, 2025 21:34
sblantipodi added a commit to sblantipodi/glow_worm_luciferin that referenced this pull request Jan 4, 2025
- ***Breaking changes***: requires `Firefly Luciferin` firmware (v2.19.3).   
- The priority of UDP packets in wireless mode has been increased to signal to the router that Luciferin traffic requires lower latency than standard packets.
- If the microcontroller is temporarily disconnected from the WiFi network, Firefly Luciferin is now able to reconnect much faster without restarting the screen capture.
- Added a 'bottom' capture option for [satellites](https://github.com/sblantipodi/firefly_luciferin/wiki/Surround-lighting-with-satellites) when the LEDs are configured to use a bottom gap.
- The [save state](https://github.com/sblantipodi/firefly_luciferin/wiki/Remote-Access#luciferin-web-interface) has been restructured. Auto-save has been disabled to prevent wear on the microcontroller's memory. Closes [#249](sblantipodi/firefly_luciferin#249).
- Arch Linux package. Note: AUR package is built from the official sources but it's currently maintained by @Ape. Closes [#246](sblantipodi/firefly_luciferin#246). Thanks @Ape for this.
- Libasound2t64 dependency prevents correct installation on some Linux distros. Closes [#253](sblantipodi/firefly_luciferin#253).
- Properly handle expired restore token on Wayland. Closes [#259](sblantipodi/firefly_luciferin#259). Thanks @Ape for the PR.
- Logging improvements. Closes [#260](sblantipodi/firefly_luciferin#260). Thanks @Ape for the PR.
- Proper config path on Linux. Config file and logs has been moved in XDG_CONFIG_HOME (~/.config/FireflyLuciferin). Old config files will be automatically moved to the new path. Closes [#261](sblantipodi/firefly_luciferin#261). 
- The snap version was crashing at startup when there were temporary files created by other instances of Firefly Luciferin on the system. Fixed.
sblantipodi added a commit that referenced this pull request Jan 4, 2025
- ***Breaking changes***: requires `Glow Worm Luciferin` firmware (v5.18.2).   
- The priority of UDP packets in wireless mode has been increased to signal to the router that Luciferin traffic requires lower latency than standard packets.
- If the microcontroller is temporarily disconnected from the WiFi network, Firefly Luciferin is now able to reconnect much faster without restarting the screen capture.
- Added a 'bottom' capture option for [satellites](https://github.com/sblantipodi/firefly_luciferin/wiki/Surround-lighting-with-satellites) when the LEDs are configured to use a bottom gap.
- The [save state](https://github.com/sblantipodi/firefly_luciferin/wiki/Remote-Access#luciferin-web-interface) has been restructured. Auto-save has been disabled to prevent wear on the microcontroller's memory. Closes [#249](#249).
- Arch Linux package. Note: AUR package is built from the official sources but it's currently maintained by @Ape. Closes [#246](#246). Thanks @Ape for this.
- Libasound2t64 dependency prevents correct installation on some Linux distros. Closes [#253](#253).
- Properly handle expired restore token on Wayland. Closes [#259](#259). Thanks @Ape for the PR.
- Logging improvements. Closes [#260](#260). Thanks @Ape for the PR.
- Proper config path on Linux. Config file and logs has been moved in XDG_CONFIG_HOME (~/.config/FireflyLuciferin). Old config files will be automatically moved to the new path. Closes [#261](#261). 
- The snap version was crashing at startup when there were temporary files created by other instances of Firefly Luciferin on the system. Fixed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs more work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants