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

Named those two vga port constants #55

Merged
merged 1 commit into from
Sep 4, 2024

Conversation

Real-Septicake
Copy link
Contributor

Description

Please include a summary of the changes and the related issue. Please also include any dependencies required for this change

Issue #

Type of change

  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)

Testing

Please describe tests that you ran to verify your changes. Provide instructions so that we can reproduce. Please also list any relevant details for your test configuration.

Test Configuration:

  • Hardware
  • OS

Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

Copy link
Member

@Sploder12 Sploder12 left a comment

Choose a reason for hiding this comment

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

Did some more digging on that Reddit thread and got to this page which describes the address and data ports for VGA in more depth. Your names are correct and good!

I think it would also be worthwhile to change the address bytes to macros as well! For example changing 0x0F to something like CURSOR_START.

Anyway, I'll approve this and let you merge your first PR! Merge when you're ready!

@Real-Septicake
Copy link
Contributor Author

I don't have write access, I can't merge

@Sploder12
Copy link
Member

Oops... I forgot...

@Sploder12 Sploder12 merged commit 7b1ac7e into makeopensource:main Sep 4, 2024
1 check passed
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.

2 participants