Skip to content

Conversation

tsatke
Copy link
Contributor

@tsatke tsatke commented May 10, 2023

That's the current implementation in my kernel, which seems to work nicely.
I'm not too sure about the configuration, so maybe pay extra attention there while reviewing.

I've added this because this crate doesn't seem to work with the new rust-osdev/bootloader 0.11.3, which starts in 1280x800x256.

@stuaxo
Copy link

stuaxo commented May 14, 2023

It looks like something happened at the top of the PR about 320x200 Vs 320x240, is that meant to be in this PR?

@stuaxo
Copy link

stuaxo commented May 14, 2023

Not my project, but there were quite a few standard resolutions smaller than 1280x800 that aren't in this, I wonder If they should also be here, or in other PRs?

(640x480x256, 800x600x256, 1024x768x256) and more unusual ones like 640x400x256

@RKennedy9064
Copy link
Member

It looks like something happened at the top of the PR about 320x200 Vs 320x240, is that meant to be in this PR?

Seems like that's fixing a copy paste error on my end. He fixed the comment to match the code.

Not my project, but there were quite a few standard resolutions smaller than 1280x800 that aren't in this, I wonder If they should also be here, or in other PRs?

(640x480x256, 800x600x256, 1024x768x256) and more unusual ones like 640x400x256

I'd love to have more modes implemented. If that's something you want to do could you make a PR for each on you implement?

@RKennedy9064
Copy link
Member

@tsatke Looks good so far. Would you mind adding a test case for this mode in vga/testing/tests/vga.rs before I approve it?

@tsatke
Copy link
Contributor Author

tsatke commented May 15, 2023

How do I run those tests locally? Seems like cargo test doesn't work, whether with build-std nor with --target nor some other way.

@tsatke
Copy link
Contributor Author

tsatke commented May 19, 2023

Ping @RKennedy9064

@RKennedy9064
Copy link
Member

How do I run those tests locally? Seems like cargo test doesn't work, whether with build-std nor with --target nor some other way.

To run the tests locally the steps are inside of .github/workflows/rust.yml. I'm not sure what OS you're running on, but you essentially just need to run each step in that file. The -name is the name of the step, run: is the command you would run and working-directory: is the directory you should be in when you run the command. The important parts are here.

      - name: "Run cargo build"
        run: cargo build

      - name: "Run cargo test"
        run: cargo test

      - name: "Install Rustup Components"
        run: rustup component add rust-src llvm-tools-preview
      - name: "Install cargo-xbuild"
        run: cargo install cargo-xbuild --debug --root binaries
      - name: "Install bootimage"
        run: cargo install bootimage --debug --root binaries

        # install QEMU
      - name: Install QEMU (Linux)
        run: |
          sudo apt update
          sudo apt install qemu-system-x86
        if: runner.os == 'Linux'
      - name: Install QEMU (macOS)
        run: brew install qemu
        if: runner.os == 'macOS'
        env:
          HOMEBREW_NO_AUTO_UPDATE: 1
          HOMEBREW_NO_BOTTLE_SOURCE_FALLBACK: 1
          HOMEBREW_NO_INSTALL_CLEANUP: 1
      - name: Install QEMU (Windows)
        run: |
          choco install qemu --version 2021.5.5
          echo "$Env:Programfiles\qemu" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append
        if: runner.os == 'Windows'
        shell: pwsh
      - name: "Print QEMU Version"
        run: qemu-system-x86_64 --version

      - name: "Print QEMU Version"
        run: qemu-system-x86_64 --version

      - name: "Build Test Kernel"
        run: cargo xbuild
        working-directory: "testing"

      - name: "Run Test Framework"
        run: cargo xtest --verbose
        working-directory: "testing"

@tsatke
Copy link
Contributor Author

tsatke commented May 23, 2023

@RKennedy9064 thanks, seems to work. Feel free to run the CI steps.

@tsatke
Copy link
Contributor Author

tsatke commented May 24, 2023

🎉

@tsatke
Copy link
Contributor Author

tsatke commented May 28, 2023

Ping @RKennedy9064

@RKennedy9064
Copy link
Member

@tsatke Looks good to me. I'll work on merging this in, updating the readme and publishing a new version.

@RKennedy9064 RKennedy9064 merged commit 5997235 into rust-osdev:master May 30, 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.

3 participants