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

Connect to DreamConn only once registered #1828

Merged
merged 4 commits into from
Feb 3, 2025

Conversation

Tails86
Copy link

@Tails86 Tails86 commented Feb 1, 2025

This is regarding discussion #1825

Problem: When no config is present, then a DreamConn connection isn't being made.

DreamConnGamepad::set_maple_port() needs to be called in order to establish connection to the device. It should always be called in GamepadDevice::Register(), even when cfgLoadInt() returns the default result. This is essentially just setting it to the same defaulted index it was already set to if it isn't in the config.

@kosekmi
Copy link

kosekmi commented Feb 1, 2025

Fix confirmed

@flyinghead
Copy link
Owner

Can you do this call in the DreamConnGamepad constructor instead?
Like here.
set_maple_port() is already called by the SDLGamepad constructor but you can call it again if you need to.
No big deal if it's not possible/easy but better to keep device-specific stuff in their own source.

@flyinghead
Copy link
Owner

actually you certainly want dreamcastControllerType to be correctly set before so here is a better place.

@Tails86
Copy link
Author

Tails86 commented Feb 2, 2025

Can you do this call in the DreamConnGamepad constructor instead? Like here. set_maple_port() is already called by the SDLGamepad constructor but you can call it again if you need to. No big deal if it's not possible/easy but better to keep device-specific stuff in their own source.

I think it would make sense to prevent multiple connect() calls, so I would feel more comfortable adding something like virtual void registered(); to GamepadDevice so the DreamConnGamepad can react to registration and only do connection attempts once registered in set_maple_port(). Does that sound like an ok compromise?

@Tails86 Tails86 changed the title Always call set_maple_port(), even when maple port isn't in config Connect to DreamConn only once registered Feb 2, 2025
@Tails86
Copy link
Author

Tails86 commented Feb 2, 2025

I pushed some suggested changes. Let me know if this seems more reasonable.

I feel the itch to more generalize things around dreamconn so parent class naming isn't specific to DreamConn and to allow for more Dreamcast-specific devices in the future (with some sort of factory). Would that be overstepping?

@flyinghead
Copy link
Owner

Sorry guys. There is probably some misunderstanding since I wasn't asking for such drastic changes.
I thought the issue was the following:
When the DreamConnGamepad is created, its base constructor (SDLGamepad) is called first, which calls set_maple_port(). At this point it's not even known which type of controller it is (DreamConn or DreamcastControllerUsb) so no connection can be made (and should not be attempted).
Since set_maple_port() isn't usually called again (except in GamepadDevice::Register() as you found out), the controller is never connected to the actual hardware. So my point was: why not calling set_maple_port() in the DreamConnGamepad constructor once the controller type is known? That way you don't have to rely on GamepadDevice::Register() calling it.

But I'm probably missing part of the story so I'm fine with the previous PR, which is much simpler.

@Tails86
Copy link
Author

Tails86 commented Feb 3, 2025

Yes, that is all correct. I'm sorry for not laying all of this out - what I was also trying to avoid is the case where the controller defaults to A, connects to A, and then the configuration changes it to B right away. I don't want it to connect multiple times in succession which would cause issues in the current implementation (@kosekmi and I spoke about fixing that in the future). I do like the current changes since it's more explicit about only connecting once during this initialization sequence.

@flyinghead flyinghead merged commit 77ed9c2 into flyinghead:dev Feb 3, 2025
15 checks passed
@flyinghead
Copy link
Owner

All right, let's go for it then.

Note that this will not prevent the maple port to change rapidly since it can be done in the UI and has immediate effect.

I'm not sure it makes sense on the hardware side of things but if changing maple port takes too much time/effort, you might want to think about decoupling the emulator's maple port from the hardware maple port. You'd need to change the the bus id in the maple packet before sending them but it shouldn't be too hard.

@Tails86
Copy link
Author

Tails86 commented Feb 3, 2025

you might want to think about decoupling the emulator's maple port from the hardware maple port

Yes, totally! It would make sense to have the connection to the serial device happen by default, and then handle the maple port ID separately. I don't think the flycast-specified ID has much merit to this connection now that you mention it 😅 🤦

A little side discussion: I've personally been trying to decide how to proceed which has caused me some implementation paralysis (and the above confusion). I'm going to brain dump here. Maybe you and @kosekmi can comment.

  • The hardware supports 2 host modes: support a single controller (host-1p) or support 4 controllers at once through one USB device (host-4p)
  • The host-1p mode accepts maple commanding for any bus ID and funnels it to the single controller, so what really only matters is the selected serial device
  • The host-4p mode still creates a single serial device and relies more on the bus ID in the command to determine which controller to send the command to
    • This bus ID must be the gamepad index, not necessarily the flycast-assigned index (unless we want the user to debug this themselves)
  • The host-4p works flawlessly on Linux, but Windows has issues enumerating all 4 controllers (it assigns each controller with the same string, even though the USB descriptor assigns a unique string for each: P1, P2, P3, P4)
    • Would have to either rely on the enumeration order that Windows assigns or use the flycast specified bus ID for this case
  • With using host-4p mode, the 4 devices must share the same serial device and command to the correct bus for their controller (instead of the assigned bus ID)
    • In order to be able to share this connection across gamepads, connection commanding would need to be serialized (probably a given anyway due to the nature of flycast)
  • I'm wondering if setting up a network on the USB device would help at all, but that actually feels more imprecise in the long run
  • I would love to be able to setup an OUT endpoint for each HID gamepad and just communicate through the SDL library, but there doesn't seem to be a way to do so unless I'm missing something
    • The SendEffect functionality looked promising, but it isn't implemented for generic devices like this (as far as I can tell)

@flyinghead
Copy link
Owner

The DreamConn maple port is special in that regard since it is set externally (by the DreamConn utility), and each port is accessible on a known network port. So when the maple port is changed, Flycast closes the current socket and opens a new one to the new network port (which may fail).
This is very different from the way your USB controller works and might be a source of confusion too.

SDL exposes the HID API so you can use it directly (SDL_hid_open, SDL_hid_read, etc.). I've used it for a PoC: 1d41069.
I don't know it this would help though and if it can be mixed with regular SDL controller handling.

@kosekmi
Copy link

kosekmi commented Feb 4, 2025

I'm wondering if setting up a network on the USB device would help at all, but that actually feels more imprecise in the long run

Not sure if this is the same idea as the following which I originally had in mind before stumbling over the serial device:

We could mimic the behaviour of DreamConn utility on the pico by exposing an ethernet device with a defined IP instead of a serial device, spinnig up a TCP socket for every controller analogue to DreamConn utility. This would certainly simplify things in Flycast.

See https://github.com/maxnet/pico-webserver for an example of a Pi Pico Ethernet device with lwip TCP sockets.

@Tails86
Copy link
Author

Tails86 commented Feb 4, 2025

I'm wondering if setting up a network on the USB device would help at all, but that actually feels more imprecise in the long run

Not sure if this is the same idea as the following which I originally had in mind before stumbling over the serial device:

We could mimic the behaviour of DreamConn utility on the pico by exposing an ethernet device with a defined IP instead of a serial device, spinnig up a TCP socket for every controller analogue to DreamConn utility. This would certainly simplify things in Flycast.

See https://github.com/maxnet/pico-webserver for an example of a Pi Pico Ethernet device with lwip TCP sockets.

Yes, this is what I was thinking. That's an interesting project! I didn't realize that Windows only supported RNDIS. I friggen hate how Windows implemented USB.

@Tails86
Copy link
Author

Tails86 commented Feb 4, 2025

SDL exposes the HID API so you can use it directly (SDL_hid_open, SDL_hid_read, etc.). I've used it for a PoC: 1d41069. I don't know it this would help though and if it can be mixed with regular SDL controller handling.

😮 That may be useful. I'll check that out. Thanks!

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