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

Improve Logging #314

Merged
merged 6 commits into from Jan 9, 2023
Merged

Conversation

asensio-project
Copy link
Contributor

@asensio-project asensio-project commented Jan 5, 2023

Hello,

This PR improves the actual logging of the bootloader.

Tasks:

  • Separate the logger from the framebuffer
  • Add support for Serial Being logger
  • Add configurations for enabling or disabling these methods of logging

Closes #306

Thanks!

@phil-opp
Copy link
Member

phil-opp commented Jan 5, 2023

Looks great already, thanks a lot! The serial printing seems to work already on the CI, for example we see the bootloader output here: https://github.com/rust-osdev/bootloader/actions/runs/3846887378/jobs/6552698281#step:11:223

Just for information: I just restarted the Windows CI job manually in the hope of triggering the timeout issue e.g. seen in #304. I suspect that the cause of that timeout could be a panic in the bootloader, which this PR should make visible.

@jasoncouture
Copy link
Contributor

Makes sense. I'll also block config.txt on this as well, once the kernel config is in place.
It may yet again be wise to wait on #302 to avoid conflicts before doing the kernel config. So, perhaps do what is currently in this PR separately, as it'll be a huge help.

Copy link
Member

@phil-opp phil-opp left a comment

Choose a reason for hiding this comment

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

Looks very good! I only left some style comments.

api/src/config.rs Outdated Show resolved Hide resolved
common/src/logger.rs Outdated Show resolved Hide resolved
common/src/logger.rs Outdated Show resolved Hide resolved
common/src/logger.rs Outdated Show resolved Hide resolved
common/src/logger.rs Outdated Show resolved Hide resolved
common/src/logger.rs Outdated Show resolved Hide resolved
@asensio-project asensio-project marked this pull request as ready for review January 5, 2023 22:59
@asensio-project asensio-project marked this pull request as draft January 6, 2023 11:40
@asensio-project asensio-project marked this pull request as ready for review January 6, 2023 19:14
@asensio-project
Copy link
Contributor Author

I can't apply the changes proposed by @jasoncouture in the linked issue because it's very difficult to refactor BIOS stages due incompatibility problems of the used crates of the logger. So, I think that this PR is ready to merge.

@phil-opp
Copy link
Member

phil-opp commented Jan 9, 2023

Thanks!

@phil-opp phil-opp merged commit 94c71d5 into rust-osdev:main Jan 9, 2023
@phil-opp phil-opp mentioned this pull request Mar 12, 2023
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.

Better logging
3 participants