Skip to content

Update kernel.cpp for C128 extra keys in C128 mode.#240

Open
nickgoodmanuk wants to merge 41 commits intorandyrossi:masterfrom
nickgoodmanuk:master
Open

Update kernel.cpp for C128 extra keys in C128 mode.#240
nickgoodmanuk wants to merge 41 commits intorandyrossi:masterfrom
nickgoodmanuk:master

Conversation

@nickgoodmanuk
Copy link

Added my proposed change for the Commodore 128 keyboards extra keys, as discussed in issue 201. This is untested as I was not able build my own kernal.

#201

Added extra keys for real Commodore 128 keyboard.

randyrossi#201
@nickgoodmanuk nickgoodmanuk changed the title Update kernel.cpp Update kernel.cpp for C128 extra keys in C128 mode. Aug 22, 2023
@randyrossi
Copy link
Owner

randyrossi commented Aug 22, 2023 via email

@nickgoodmanuk
Copy link
Author

nickgoodmanuk commented Aug 22, 2023

> I will make a build with these changes in and put it up as a test build in a directory on my site. Can you ping me if you don't hear back from me in a couple days? I'm consumed by work right now but I will try to do this so you can test the changes. Then we can merge this in and do a release.

Will do, Thankyou. I know you are very busy, as am I haven't had chance to address it further until now. I appreciate it.

@nickgoodmanuk
Copy link
Author

@randyrossi Gentle reminder about a test build. No rush :)

Copy link
Owner

@randyrossi randyrossi left a comment

Choose a reason for hiding this comment

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

Please see review comments.

kernel.cpp Outdated
{KEYCODE_F5, KEYCODE_e, KEYCODE_t, KEYCODE_u, KEYCODE_o, KEYCODE_LeftBracket, KEYCODE_Equals, KEYCODE_q},
{KEYCODE_Insert, KEYCODE_LeftShift, KEYCODE_x, KEYCODE_v, KEYCODE_n, KEYCODE_Comma, KEYCODE_Slash, KEYCODE_Escape},
};
#else if defined(RASPI_C128)
Copy link
Owner

Choose a reason for hiding this comment

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

This should be #elif defined(RASPI_C128)

Fails to compile as-is. I'll fix locally for a build but please change before I merge this in.

kernel.cpp Outdated
{KEYCODE_Insert, KEYCODE_LeftShift, KEYCODE_x, KEYCODE_v, KEYCODE_n, KEYCODE_Comma, KEYCODE_Slash, KEYCODE_Escape},
};
#else if defined(RASPI_C128)
static long kbdMatrixKeyCodes[11][8] = {
Copy link
Owner

Choose a reason for hiding this comment

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

These dimensions are backwards from the data provided. Also, if the dimension is going to change then the kbdPA iterator later on in this file would have to change to 0->10 but it is still only 0-7. Then the gpioPins array would also have to be adjusted for the scan to actually include the extra 3 pins. gpioPins[8] and gpioPins[9] and gpioPins[10] would have to become the three new PA lines. Then the remaining would have to shift up. I don't think this change is sufficient to do what you want it to do. There would be more work elsewhere and a bunch of conditionals to extend the PA lines only for the C128 builds.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks ever so much for looking. I will try and fix based on your comments.

@randyrossi
Copy link
Owner

randyrossi commented Aug 31, 2023 via email

@nickgoodmanuk
Copy link
Author

Changes made to pull request now I have been able to build myself.

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