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

Specific Version of The64Maxi Keyboard doesn't work in Circle 48 #531

Closed
Chris-Hal opened this issue Jan 6, 2025 · 36 comments
Closed

Specific Version of The64Maxi Keyboard doesn't work in Circle 48 #531

Chris-Hal opened this issue Jan 6, 2025 · 36 comments

Comments

@Chris-Hal
Copy link

Chris-Hal commented Jan 6, 2025

Hello Rene,
I would like report one issue with the Circle library in combination with Randy Rossi BMC64 project.
https://github.com/randyrossi/
To be honest, Randy suggested to me, to open an issue, because I was already in contact and our findings seems to be an incompatible USB device in combination with the circle library. Additionally there are already users, which are using his BMC64 on Raspberry Pi 2/3 together with a modified TheC64 Maxi hardware.
https://retrogames.biz/products/thec64/

From the TheC64 Maxi only the USB keyboard will be used to the Pi and the internal USB hub of the Maxi, so all together becomes a perfect replica of an C64. So did I with an RPI2 B version. Sorry if I bother you wiith such details in case you know this already.

When I stick "TheC64" Keyboard to the Raspberry Pi and start his BMC64 which comes Originally with circle 40 the Pi will stuck as visible in this screen.
Maxi_keyb_connected_IMG20250106092929_lq

If I connect no keyboard at all to the Raspberry Pi, than BMC with Circle boots normally but without Keyboard input.
maxi_keyb_disconnected_IMG20250106093031_lq

The 3rd option was to connect any noname Wireless Keyboard dongle to the Raspberry which looks like this and typing is possible.
USB Dongle_IMG20250106125053_lq

Randy created for me a test enviroment with Circle library 48 but even here the boot process stuck when detecting the TheC64 keyboard.
Circle48_Maxi_KB_connected_IMG20250106145124

As I mentioned in the beginning there are already users, which are using the BMC64 with Circle library together with TheC64 Keyboard without issues, so maybe there is a new revision? Additionally I can connect my BMC64 keyboard to a PC running LMDE6 or I can use a Raspian based distribution on the Rpi2 and both detects the keyboard fine and let me use the TheC64 Keyboard, so there seems to be no hardware issue but an incomatibility within the Circle library and I would be really happy if you can look into it, because as I said, BMC64 with an Raspberry Pi and the C64 replica is the perfect combintion for Retro Gamer like me.

Of course let me know, if I can assist you by your investigation. I am happy to do some testing if needed.
Thanks a lot for your time and your project which make this possible at all.

Best regards
Christian

@rsta2
Copy link
Owner

rsta2 commented Jan 7, 2025

I guess the keyboard is connected via the internal USB hub of the The64Maxi device to the Raspberry Pi 2? Unfortunately the Circle USB hub driver has probably a problem with this hub. Is it possible to bypass it?

@Chris-Hal
Copy link
Author

Chris-Hal commented Jan 7, 2025

Hello Rene,
thanks for your fast answer. No the wiring is different. The USB keyboard and the USB hub has been both equipped by me with a Micro USB breakout boards of this type:
image

So normal USB-A to Micro-USB cables connect each device directly with the PI. I can also confirm that the USB Hub is not the problematic device or cause, because the wireless USB kyeboard dongle is connected through the Maxi hub with the PI and the wireless keyboard works without issues. It is really the Maxi keyboard which cause the issue.

Best regards
Christian

@rsta2
Copy link
Owner

rsta2 commented Jan 8, 2025 via email

@Chris-Hal
Copy link
Author

Chris-Hal commented Jan 8, 2025

Hello Rene,
thanks again!

External Hub is not connected, therefor the device list is shorter than in the screen above, but still no function.
image
cmdline.txt looked like this:

fast=true machine_timing=pal-hdmi scaling_params=384,240,1152,720
usbpowerdelay=1500
DO NOT EDIT - This file is no longer edited by hand.
Use the machines.txt to set both config.txt and cmdline.txt params for each machine.
Refer to README.md for options.

Btw. when I connect the keyboard to my LMDE machine, lsusb identifies the keyboard as
Bus 003 Device 012: ID 1c59:0099 HideLink THEC64 Keyboard
and it is working.

@rsta2
Copy link
Owner

rsta2 commented Jan 9, 2025

Oops, I forgot to mention, that the usbpowerdelay=1500 option must be appended to the first line of the file cmdline.txt, delimited with a space character. Only the first line is considered for the command line.

@Chris-Hal
Copy link
Author

Chris-Hal commented Jan 9, 2025

:) no problem, it is easy repeatable. Now the first line looks like this, but unfortunately the screen is the same as uploaded before, there is no difference in the final result except the time between "Using device..." and "Device xxxx found" has been increased to 1,5 seconds.

fast=true machine_timing=pal-hdmi scaling_params=384,240,1152,720 usbpowerdelay=1500

@rsta2
Copy link
Owner

rsta2 commented Jan 10, 2025

Thanks for testing! Attached there is another test kernel image. Can you please test this in the same configuration and take a photo of the last log lines, displayed before the system hangs. You only need to replace the file kernel7.img with the file extracted from the .zip file.

kernel7.zip

@Chris-Hal
Copy link
Author

Thanks for your ongoing support. So I did the test, just the Maxi keyboard connected to the Pi, is this the expected output?
IMG20250110143934_lq

@rsta2
Copy link
Owner

rsta2 commented Jan 10, 2025

Yes, that's the output, I expected. Thanks for testing! It hangs in CUSBDevice::Initialize(). Here is another test, which should show, where it hangs in this method. Same procedure as before.

kernel7.zip

@Chris-Hal
Copy link
Author

Chris-Hal commented Jan 12, 2025

Thanks again, Here is the output from this Kernel. The output is different than before, so I cross my fingers, that it can help you in this matter. Btw. would it be helpful for you because of the situation that you don't have the hardware, that I try to install a software like FreeUSBAnalyzer on my computer and try to gather some information about the Maxi Keyboard device directly?

I found this software for USB debugging and I was considdering, if this can be helpful?
https://freeusbanalyzer.com

MaxiKb_an_PI2_IMG20250112104443_lq

@rsta2
Copy link
Owner

rsta2 commented Jan 13, 2025

Thanks for testing! The system hangs, when it tries to fetch the configuration descriptor from the device. I will provide another lower layer dump test kernel soon.

I don't think that this tool would help that much, because you cannot check the communication between the RPi and the keyboard with it. This would require a hardware sniffer.

@Chris-Hal
Copy link
Author

Alright, thanks for your explanation. I got the point, that the expected layer, where the problem happens can not be logged by such software tool, but you know much more than me about this, so I am in good mood, that you are able to fix it, because I would be lost :-)

@rsta2
Copy link
Owner

rsta2 commented Jan 13, 2025

I don't know, if we can fix this, but I try. ;) Here is another test kernel image appended. It now logs the status of the USB transactions, and when it hangs for 500ms it stops and displays the status of the last 20 transactions. I hope I can found something in this.

kernel7.zip

@Chris-Hal
Copy link
Author

Thanks for your again fast response with a new kernel. I made 2 screenshots. The first RPI2 with Maxi keyboard connected . It stuck on "Getting configuration descriptor" without further output. cmdline.txt untouched as before.
The second is the Pi only without anything connected to USB as reference, how it looks like without Maxi keyboard.
Maxi_KB connected_lq

Without_KB_just_pi_lq

@Chris-Hal
Copy link
Author

Hello Rene,
I had some time and simply tested Free USB Analyzer with the Maxi Keyboard and maybe one of these details can help. Just in case.
2025-01-13 15_00_56-Free Device Monitoring Studio
2025-01-13 15_02_35-Free Device Monitoring Studio
2025-01-13 15_03_12-Free Device Monitoring Studio

@rsta2
Copy link
Owner

rsta2 commented Jan 13, 2025

Thanks for testing and the info from the Free USB Analyzer! How it seems, I was wrong with my assumption, that it hangs, when trying to fetch the configuration descriptor. Instead the wTotalLength field in the descriptor seems to be wrong. The descriptor is 3 * 9 = 27 bytes long, not 66 as specified. I guess this caused the descriptor parser to hang. There is a new kernel test image attached, which should fix this problem and displays the configuration descriptor.

kernel7.zip

@Chris-Hal
Copy link
Author

Chris-Hal commented Jan 13, 2025

I am thrilled, thank you Rene, good news, you are one step further
Lot of new output now.
Maxi_KB connected_progress_lq

@rsta2
Copy link
Owner

rsta2 commented Jan 13, 2025

The configuration descriptor is definitely not according to the USB spec. There is a descriptor with zero length in it, so the parser never came to an end. Now I use this as a stop condition. Another kernel image attached, hopefully the last one.

kernel7.zip

@Chris-Hal
Copy link
Author

Chris-Hal commented Jan 13, 2025

Hello Rene, this is a huge step forward, please have a look at the output, also when I type on the keyboard the cursor is moving around. I don't see characters or numbers,(Should i?) but at least the cursor is moving. So the Kernel reacts to keyboard presses!
Pi_with_Maxi_Super_lq

@rsta2
Copy link
Owner

rsta2 commented Jan 14, 2025

OK, now that the device is detected, the question is, if the key reports are in the right format and how is the mapping from physical key codes to logical characters. Please run the attached kernel test image and press some keys. The program displays the physical key codes. Normally the A key has the code 0x04, B has 0x05 and so on. You can also press multiple keys at once. The program should be able to display multiple keys. Please check, if there is any system in the key codes, which makes sense. Each key must have a unique key code. The modifier keys (e.g. Shift, Ctrl) are displayed separately in the modifiers bitmap. Thanks!

kernel7.zip

@Chris-Hal
Copy link
Author

Thanks Rene, you are great! I can make today a brief check only, but it looks very good A=0x04 B=0x05 Y=0x1C Z=0x1D 1=0x1E 9=0x26 as you assume.

I know from BMC64, that I have to load a special Keyboard config file for getting the TheC64 Maxi keyboard fully to work in BMC64 which is included there, I know for example the Shift and Commodore keys needs a special configuration in BMC64 or the F1/F3/F5/F7 of the C64 Keyboard which gives back 0x3A/0x3C/0x3E/0x40 are fine, but I cannot get F2/F4/F6/F8 to work yet, which are Shift+F-Keys which probably will result in 0x3B/0x3D/0x3F/0x41.
But I believe this is a matter of the correct keyboard mapping file in BMC64 later and has nothing to do yet with the Kernel itself, but this is just my humble assessment of the actual situation.

rsta2 added a commit that referenced this issue Jan 15, 2025
The HideLink THEC64 USB keyboard provides a configuration descriptor,
which contains a descriptor with length 0. This caused the configuration
descriptor parser to hang. Now a length of 0 is used as stop condition.

Issue #531
@rsta2
Copy link
Owner

rsta2 commented Jan 15, 2025

Thanks! This looks good. Now the question is, how can this be included into BMC64, which uses an other release of Circle (Step40?). You have to apply the patch from commit 1bb0882, listed above, in any case, so that the USB configuration parser works. Furthermore the keyboard mapping should be adapted, as you already noted.

@Chris-Hal
Copy link
Author

Hello Rene,
let me pass your feedback to Randy, he as the project leader and programmer of BMC64 suggested to me to contact you, so I am very hopeful, that he has the interest and find the time and will implement your chances into BMC64 , since you have sorted out all the hardware related issues.
for the time being,I want to thank you for your patient, your support and your time to investigate and solve my reported issue, I really appreciate that and I feel special.

Thanks and best regards
Christian

@Chris-Hal
Copy link
Author

Chris-Hal commented Jan 16, 2025

Hello Rene,
Randy added the patch already , the keyboard shows characters now and works from the functional point, but the output is not the expected, but there is one thing, which I didn't understand before and I would like to ask you again for your support. This Kernel from you, I refer below, shows me the keycode and modifier. I didn't understand the term "modifier" before, so I did not recognize the difference. When I connect a normal keyboard, and press A, I get modifier 0 and keycode 0x04 if I press shift + A, the modifer switches to 1 and Keycode stays 0x04

When I do the same with the Maxi keyboard for A I get alwyas modifier 1 and for A the keycode 0x04, if I press and hold shift, a new line appear with modifier 1 only and addionally to Shift pressing A results to a new line again, but modifier remains 1 again with keycode 0x04. So there happens something in the kernel when I press "Shift" or "Commodore" key behaves the same I mean I see a new line with modifier 1 without keycode, , but I havn't seen anything else except modifier 1 and the keycode is similar to what a normal keyboard will output but some keys opens a new line with modifier 1 only without outputting any keycode.

Is this behaviour something which has to be adressed in the Circle library or is the different feedback from the Maxi keyboard something which can be handled with keyboard mappings in BMC64? I tested all buttons, but modifier remains always 1, pressing certain keys like right shift or left shift or commodore makes a new line with modifier 1 only, without any keycode visible or if another button will be pressed, I get modifer 1 and a keycode

Thanks and best regards
Christian
IMG20250116105028_lq

OK, now that the device is detected, the question is, if the key reports are in the right format and how is the mapping from physical key codes to logical characters. Please run the attached kernel test image and press some keys. The program displays the physical key codes. Normally the A key has the code 0x04, B has 0x05 and so on. You can also press multiple keys at once. The program should be able to display multiple keys. Please check, if there is any system in the key codes, which makes sense. Each key must have a unique key code. The modifier keys (e.g. Shift, Ctrl) are displayed separately in the modifiers bitmap. Thanks!

kernel7.zip

@rsta2
Copy link
Owner

rsta2 commented Jan 16, 2025

The Circle USB keyboard driver expects the keyboard to behave like it is specified in the USB Device Class Definition for Human Interface Devices. According to this (section 8.3 on pg. 56) the modifiers byte is a bitmap, where each bit belongs to one of the modifier keys (Ctrl, Shift, Alt). Please also see pg. 60 in this document, where each byte of the 8 bytes report, sent by the keyboard, is described. The Circle driver implements the boot protocol for USB keyboards with the USB interface 3-1-1.

If the modifier byte is always 0x01, that would mean, the left Ctrl key is always pressed. This is a wrong behavior according to this spec. I don't know, how BMC64 handles the keyboard. Perhaps it could filter this out. But the question remains, how the Maxi keyboard reports the modifier keys, if this byte is always 0x01?

@Chris-Hal
Copy link
Author

Chris-Hal commented Jan 16, 2025

Hello Rene,
I believe I found something. I analysed the packages coming from a PC keyboard and compared it with the Maxi keyboard.

PC keyboard provides always 8 byte in a package, when I press a button here for example SHIFT-Key, a key, shift+a keys

Only Shift Key:
02 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00
........
only a key
00 00 04 00 00 00 00 00
00 00 00 00 00 00 00 00

shift +a
02 00 00 00 00 00 00 00
02 00 04 00 00 00 00 00
02 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00

If I use the Maxi keyboard, I will receive always 9 byte per package, same example SHIFT-Key, a key, shift+a keys. they are basically identical but there is always a leading 0x01 at the beginning for every button on the Maxi keyboard. I checked really every button and every one has this additional 0x01 at the beginning.

Only Shift Key:
01 02 00 00 00 00 00 00 00
01 00 00 00 00 00 00 00 00
........
only a key
01 00 00 04 00 00 00 00 00
01 00 00 00 00 00 00 00 00

shift +a
01 02 00 00 00 00 00 00 00
01 02 00 04 00 00 00 00 00
01 02 00 00 00 00 00 00 00
01 00 00 00 00 00 00 00 00

is it necassary to change the code that the leading 0x01 will be cut, if a 9 byte message from the keyboard will be received?

Thanks for your time and best regards
Christian

@Chris-Hal
Copy link
Author

Chris-Hal commented Jan 16, 2025

Hello Rene,
sorry I didn't realize that you already replied, before I have send my finding. I truely believe that you working accordingly to the USB specifications. This is nothing where I have a kind of knowledge and even my programming skills are very limited, so I am just trying to understand the failure and try to find explanations, which hopefully can lead to an (individual?) solution even I believe every recent TheC64 Maxi uses will run into the same trouble nowadays.

From my former post, I guess the manufacturer added the 9th byte at the beginning to obfurscate the protocol or they wasn't able to make it right, I don't know. :-) Easy enough for the manufacturer to revert it in their software but worse enough, that the hardware doesn't work in other situations like BMC64. very bad bevahiour from the manufacturer, if I am right, but I have no other explanation why a manufacturer of the keyboard should do such thing.

My humblest idea was if a HID device like the Maxi sends 9 byte per package instead the 8 by definition, filter the leading 01 and process the remaining 8 bytes as usual because they seems to be accordingly to the USB specifications. If this is feasible, in your interest or intended by you, I cannot judge because I miss the needed skills for that and I don't know,
how far your help would go.

Thanks again and best regards
Christian

The Circle USB keyboard driver expects the keyboard to behave like it is specified in the USB Device Class Definition for Human Interface Devices. According to this (section 8.3 on pg. 56) the modifiers byte is a bitmap, where each bit belongs to one of the modifier keys (Ctrl, Shift, Alt). Please also see pg. 60 in this document, where each byte of the 8 bytes report, sent by the keyboard, is described. The Circle driver implements the boot protocol for USB keyboards with the USB interface 3-1-1.

If the modifier byte is always 0x01, that would mean, the left Ctrl key is always pressed. This is a wrong behavior according to this spec. I don't know, how BMC64 handles the keyboard. Perhaps it could filter this out. But the question remains, how the Maxi keyboard reports the modifier keys, if this byte is always 0x01?

@randyrossi
Copy link

If it's just a matter of filtering out the unwanted leading byte, I can make a change to my patched circle library. I might need some help with the location in the codebase to do that though. However, i"m wondering how the keyboard works on your PC? Or does it also exhibit the same incorrect behavior?

@rsta2
Copy link
Owner

rsta2 commented Jan 16, 2025

No problem, this makes sense. The 0x01 is the Report ID, which is normally left out in the boot protocol. I will provide a modified USB keyboard driver soon, because it's not possible to handle this in BMC64 without driver support.

@Chris-Hal
Copy link
Author

Hello Rene,
this sounds great, thanks a lot for your ongoing help. Same for you you Randy and thank you for joining in

Best regards
Christian

@rsta2
Copy link
Owner

rsta2 commented Jan 16, 2025

You are welcome. Here comes a first version of the patch for the USB keyboard driver, which should work with the THEC64 keyboard and a test kernel image below. Please let me know, if it works.

diff --git a/include/circle/usb/usbkeyboard.h b/include/circle/usb/usbkeyboard.h
index 80efc18..373b0d2 100644
--- a/include/circle/usb/usbkeyboard.h
+++ b/include/circle/usb/usbkeyboard.h
@@ -2,7 +2,7 @@
 // usbkeyboard.h
 //
 // Circle - A C++ bare metal environment for Raspberry Pi
-// Copyright (C) 2014-2023  R. Stange <[email protected]>
+// Copyright (C) 2014-2025  R. Stange <[email protected]>
 // 
 // This program is free software: you can redistribute it and/or modify
 // it under the terms of the GNU General Public License as published by
@@ -68,6 +68,8 @@ private:
 	static boolean FindByte (const u8 *pBuffer, u8 ucByte, unsigned nLength);
 
 private:
+	unsigned m_nReportSize;
+
 	CKeyboardBehaviour m_Behaviour;
 
 	TKeyStatusHandlerRawEx *m_pKeyStatusHandlerRaw;
diff --git a/lib/usb/usbkeyboard.cpp b/lib/usb/usbkeyboard.cpp
index aaff99f..05fe2fa 100644
--- a/lib/usb/usbkeyboard.cpp
+++ b/lib/usb/usbkeyboard.cpp
@@ -2,7 +2,7 @@
 // usbkeyboard.cpp
 //
 // Circle - A C++ bare metal environment for Raspberry Pi
-// Copyright (C) 2014-2023  R. Stange <[email protected]>
+// Copyright (C) 2014-2025  R. Stange <[email protected]>
 // 
 // This program is free software: you can redistribute it and/or modify
 // it under the terms of the GNU General Public License as published by
@@ -32,7 +32,8 @@ static const char FromUSBKbd[] = "usbkbd";
 static const char DevicePrefix[] = "ukbd";
 
 CUSBKeyboardDevice::CUSBKeyboardDevice (CUSBFunction *pFunction)
-:	CUSBHIDDevice (pFunction, USBKEYB_REPORT_SIZE),
+:	CUSBHIDDevice (pFunction),
+	m_nReportSize (USBKEYB_REPORT_SIZE),
 	m_pKeyStatusHandlerRaw (0),
 	m_pKeyStatusHandlerRawArg (0),
 	m_bMixedMode (FALSE),
@@ -56,7 +57,17 @@ CUSBKeyboardDevice::~CUSBKeyboardDevice (void)
 
 boolean CUSBKeyboardDevice::Configure (void)
 {
-	if (!CUSBHIDDevice::ConfigureHID ())
+	// The HideLink THEC64 keyboard sends the report ID in the first byte,
+	// which has to be ignored.
+	const TUSBDeviceDescriptor *pDeviceDesc = GetDevice ()->GetDeviceDescriptor ();
+	assert (pDeviceDesc != 0);
+	if (   pDeviceDesc->idVendor == 0x1C59
+	    && pDeviceDesc->idProduct == 0x99)
+	{
+		m_nReportSize++;
+	}
+
+	if (!CUSBHIDDevice::ConfigureHID (m_nReportSize))
 	{
 		CLogger::Get ()->Write (FromUSBKbd, LogError, "Cannot configure HID device");
 
@@ -172,6 +183,14 @@ boolean CUSBKeyboardDevice::SetLEDs (u8 ucStatus)
 
 void CUSBKeyboardDevice::ReportHandler (const u8 *pReport, unsigned nReportSize)
 {
+	// Ignore report ID, if it is included.
+	if (   pReport != 0
+	    && m_nReportSize > USBKEYB_REPORT_SIZE)
+	{
+		pReport++;
+		nReportSize--;
+	}
+
 	if (   pReport == 0
 	    || nReportSize != USBKEYB_REPORT_SIZE)
 	{

kernel7.zip

@Chris-Hal
Copy link
Author

Hello Rene,
you made it! Phantastic and sooo fast. It works, I can see now, that the modifier changes his value accordingly to the Shift,Commodore or Control key, which I press. I don't know what to say, thank you very very much!

Best regards
Christian

@rsta2
Copy link
Owner

rsta2 commented Jan 16, 2025

OK, great, Christian! Thanks for testing.

@randyrossi Please let me know, if you need additional changes to include this patch into Step40.

@randyrossi
Copy link

I patched the change in and you can try a new C64 kernel here:

https://accentual.com/bmc64/downloads/usb-maxi/kernel7.img

randyrossi added a commit to randyrossi/bmc64 that referenced this issue Jan 16, 2025
@Chris-Hal
Copy link
Author

Chris-Hal commented Jan 16, 2025

Hello Rene,
everything is done. You have found all the issues existing with the Maxi keyboard and solved them The problem is no longer a problem.

Thanks again.

https://github.com/randyrossi/bmc64/issues/288#issuecomment-2596823169

rsta2 added a commit that referenced this issue Jan 17, 2025
The HideLink THEC64 keyboard sends an additional leading report ID,
which has to be ignored.

Issue #531
@rsta2
Copy link
Owner

rsta2 commented Jan 17, 2025

Hi Christian, you are welcome and thanks for all that testing. I'm glad, that we were able to solve this. Thanks also to Randy for quickly including this into BMC64.

Rene

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

No branches or pull requests

3 participants