BlitterStudio / amiberry

Optimized Amiga emulator for Linux/macOS
https://amiberry.com
GNU General Public License v3.0
660 stars 89 forks source link

Implement support for uaeserial.device #723

Closed midwan closed 2 years ago

midwan commented 3 years ago

Implement support for using the uaeserial.device as in WinUAE.

Preliminary work started on the uaeserial branch.

gsfare commented 3 years ago

Hey guys, I’ll clone the branch this weekend and take a look at those stubs and let you know here if I make progress.

midwan commented 3 years ago

Awesome, thanks again for helping out! :)

gsfare commented 3 years ago

I did some initial fleshing out of the stubs here,

https://github.com/gsfare/amiberry/tree/uaeserial_stubs

I think there is still a fair amount of work to do though. I don't yet know what the uaeserial device actually does for at the moment. There is also some Win32 style sync/async handling that will need looking into. This is specifically the function uaeser_trigger() and thread uaeser_trap_thread(). Whether it's needed in a POSIX style system I am again not certain as yet. I think the emulation of this (Win32->POSIX) may require the use of something like epoll().

Also a decision should be made on which physical serial device to open. Win32 is simply using COMM 0..n but POSIX uses names such as /dev/ttyS 0..n for onboard/virtual devices but also for example /dev/ttyUSB 0..n for USB style serial dongles.

midwan commented 3 years ago

@gsfare Thanks for looking into this! Enabling this option should inject the virtual device name uaeserial.device in the system, which can be addressed from system-friendly applications, that will use it as a serial port/device. It uses internal "traps" to handle the hooks from inside AmigaOS to the emulator and back, but most of that logic is already implemented I think.

Regarding the serial device to use, we should of course keep using the one the user specifies in the field you added (e.g. /dev/ttyUSB0). If none is specified there, then it shouldn't be possible to use/enable these options at all, and that's something I can take care of afterwards if you want (GUI stuff).

Hope it's a bit more clear now?

midwan commented 3 years ago

Also, regarding some of the threading stuff: uae_set_thread_priority, uae_sem_init, uae_start_thread, uae_sem_post and uae_sem_wait are all implemented in Amiberry and used elsewhere in the code, already. But I'm not sure how to handle those CreateEvent, SetEvent, WaitForSingleObject etc. calls, or if we actually need them in the implementation.

Once scenario to test this out, once it builds, is to use a system application that would allow you to configure the serial device name, in AmigaOS. I have a graphics tablet that uses a serial connection to a real Amiga, for example. The driver is Formaldihyd, by Chris Hodges (https://aminet.net/package/driver/input/FormAldiHyd). I could test it with that.

Currently, using serial.device with it fails to communicate - but I believe if we implement uaeserial.device and use that, it will work (it does on WinUAE).

gsfare commented 3 years ago

It does make more sense now thank you. I'll see if I can find a way to reproduce a test scenario. The code does at least build....I doubt it'll work first time but you never know!

I think the CreateEvent, SetEvent stuff is used to check when data is both available and has been actually written to the serial device. It 'shouldn't be needed since the block char device in Linux should handle it automatically. It's not so easily supported in Posix environments but there are ways to achieve the same. If it doesn't work as is I'll see if I can develop something similar on the Linux side as Windows has.

midwan commented 3 years ago

@gsfare Did you get a chance to do any more tests on this, please? In my tests, it didn't seem to work so far (uaeserial.device could not be opened from a test program trying to access it).

gsfare commented 3 years ago

Hey @midwan

Apologies for delay in getting back to you. I'll have another look into this. Right now I don't have any scenario to recreate the WinUAE use case so I need to create this first. Then I can work out how it's meant to work :-)

It should be trivial to implement in Amiberry after that.

midwan commented 3 years ago

@gsfare No worries, take your time. I'm focusing on the last remaining glitches for the 3.4 release, which will take some time to fix it seems. If you need any assistance in setting up a test scenario, perhaps I can help!

gsfare commented 3 years ago

I spent some time this week to see if I could get uaeserial.device working under WinUAE.

For my own sanity.....this is just a AmigaOS specific feature isn't it? Confirming this would at least minimise a lot the potential test scenarios.

I did manage to get it to open at least in AmigaOS but haven't been able to get anything meaningful out of the serial port yet (just garbage). I'll dig some more and enable the debug options in WinUAE. See if it's doing something weird or not.

Ideally a terminal inside the emulator and a terminal running on a desktop PC would be the ideal test scenario.

midwan commented 3 years ago

@gsfare Yes, this enables an AmigaOS "device" with the same name, which can be addressed from OS friendly apps, to send/receive info over the configured hardware serial port. Similar to other uae devices, like uaenet.device (for networking), uaescsi.device (for HDD access) etc.

gsfare commented 3 years ago

Thanks for confirming.

I have a working test scenario now. Let me see if the implementation is still trivial ;-)

gsfare commented 3 years ago

@midwan

First the good news. I have had some success today getting this to work in Amiberry.

We'll need to decide how we translate the dev unit from within AmigaOS to a serial device on the host emulator. In WinUAE...because it is windows, all serial ports are named COM0.....n, regardless of whether or not the device is USB or a built in device (where n is the dev unit in AmigaOS). Whereas in Unix bases systems the serial device can be ttyS0 for a built-in device or ttyUSB0 for a external device for instance.

The actual drop down list defining the port to use in the configuration in WinUAE is only for the emulating the default device built into the Amiga and has no relation to the host device that is configured when used by uaeseiral.device (to emulate multiport serial expansions in the Amiga). I think the best option would be to have a additional text entry in the configuration where a prefix can be specified for both use cases. One that is mapped for the default serial device and one that is used to derive the port mappings (within the host) used for uaeserial.device. This is also why the "shared" config button is available in WinUAE.

Happy to hear your thoughts and hopefully this makes sense!

Also....the Windows async stuff in parser.cpp is horrendous for this so translating it correctly may take a little longer ;-)

midwan commented 3 years ago

@gsfare Thanks for you help with this :)

I'm not sure I understand why we need 2 entries to pick the serial port used. I imagine in most cases we'll only have 1 serial port connected anyway, probably with a USB adapter. Can't we use the same device specified in the config/GUI for both features?

gsfare commented 3 years ago

@midwan

We could use the same base device name as the one given in the config. This just provides the limitation of only being able to use one device base name type such as /dev/ttyS(n) OR /dev/ttyUSB(n) but not both. Multiple serial ports are useful on the Amiga for midi hardware as I understand it. Anyway...that's the easy bit and can be decided any time in the future. I'll see if I can get this working reliably first as I am debugging some race conditions ;-)

midwan commented 3 years ago

@gsfare Yeah, let's keep it at 1 port only for now, and if the need arises in the future, we can add it as a separate option. I suspect not a lot of people (if any) will need it. ;)

midwan commented 3 years ago

@gsfare How's it going? Any progress? I'm starting a testing round with various beta testers, so I was wondering if I should wait a bit more to perhaps have this area included in the tests as well?

gsfare commented 3 years ago

Hey @midwan

Progress is slow at the moment. I have something working....but it is not very robust. Good enough for debugging purposes etc. but file transfers are unreliable. I tested this on WinUAE in the same fashion and it is also unreliable on there too so maybe it's just the way it is. I wouldn't be comfortable releasing it like it is. My feeling is there is some heavy lifting to do to make it work well. I'll tidy up what I have and then it's there for review at least.

midwan commented 3 years ago

OK, thanks for the update! :)

midwan commented 3 years ago

@gsfare Ping! Just wondering if you had a chance to do anything further regarding this?

midwan commented 3 years ago

@gsfare Any updates on this? I understand if you're busy, just need a rough idea of where we're at, in order to plan ahead for the upcoming release... :)

gsfare commented 3 years ago

Apologies for this. No, I have not been able to work on this further. I just dug out my old repo and checked in what I had, this is the latest commit.

https://github.com/gsfare/amiberry/tree/7fceaf21552b3749c9ce29f5bbbefb4d3105cc15/src

I think you should be able to access this?

I remember it was "kind of working" but was not very reliable. It was fine for typed text between two terminals but not for file transfer. (I also discovered that WinUAE is also a bit flaky for that so maybe it works fine ;-))

Just for note I did hack in a fixed tty device name (number is dynamic) in uaeserial_open(), file parser.cpp while I was debugging. So just something to note in case someone can make use of what is there.

midwan commented 3 years ago

@gsfare Thanks, I've merged your updates in the uaeserial branch for now, and made sure it compiles. It's still WIP, so I won't merge it back to dev or master yet, until we've had more time to look at it.

I noticed that the serial_read_thread and serial_write_thread both have an infinite while(1) loop - is that intentional, or only for testing?

gsfare commented 3 years ago

It's temporary ;-) ...but yes it's on purpose. The Windows implementation uses re-entrant sync code that does not map so well to Unix systems. The infinite do/while loops in this case are controlled by semaphores which are triggered by data on the physical port and data coming from the emulated 68k serial controller so the loops don't run freely. Absolutely agree that those threads should exit gracefully when the emulated port is torn down. I think this is where the issues lay as I just couldn't get those asynchronous functions to play nicely enough with the physical hardware the way it is currently.

midwan commented 2 years ago

Merged in dev, deleted separate branch. We can continue any improvements there, in the future.