DhrBaksteen / ArduinoOPL2

Arduino library for use with the OPL2 board (YM3812) and OPL3Duo (YMF262)
MIT License
198 stars 39 forks source link

SerialIface: reduce protocol to 2-byte commands #24

Closed vincentbernat closed 4 years ago

vincentbernat commented 6 years ago

Delay are computed on the client-side instead of the server-side. This is less precise but this makes the protocol less chatty and able to handle more complex tunes, like the opening of X-Wing. We use a semaphore to represent the state of the buffer on the Arduino and a separate thread handles acknowledgments.

I have not a good enough ear to know if there is a notable difference in the way tunes are rendered. I didn't find any tune not able to cope with a 256-byte buffer.

Related to #20

vincentbernat commented 6 years ago

I have pushed a second commit using multiprocessing instead of threading. This way, the ack thread cannot slow down the main thread.

vincentbernat commented 6 years ago

I added yet another commit to compensate for various processing delays in Python code. Again, not sure if there is any difference.

If we wanted to go even further, we could spawn another process just to send opcodes to the serial port. The process would take opcodes with delays from a queue (multiprocessing.Queue), send and sleep. Just like what was done directly on the Arduino until now. Would it make sense to do that?

DhrBaksteen commented 6 years ago

I only have limited Python knowledge, so please allow me some time to digest this...

DhrBaksteen commented 6 years ago

It seems to have some issues with spawning a new thread as far as I can tell. I get the following when trying to play a vgm tune:

$ python play.py COM8 zero_wing.vgm Play "zero_wing.vgm" on COM8 (Ctrl-C to stop) Traceback (most recent call last):LATE/TX: 0 LATE/CPU: 0 Tx: 42 55 46 3f 0a File "play.py", line 78, in <module> handle_arguments() File "play.py", line 66, in handle_arguments device = opl.ArduinoOpl(portname) File "C:\ArduinoOPL2\examples\SerialIface\opl.py", line 37, in __init__ self.ack_thread.start() File "C:\Users\j_maa\AppData\Local\Programs\Python\Python36\lib\multiprocessing\process.py", line 105, in start self._popen = self._Popen(self) File "C:\Users\j_maa\AppData\Local\Programs\Python\Python36\lib\multiprocessing\context.py", line 223, in _Popen return _default_context.get_context().Process._Popen(process_obj) File "C:\Users\j_maa\AppData\Local\Programs\Python\Python36\lib\multiprocessing\context.py", line 322, in _Popen return Popen(process_obj) File "C:\Users\j_maa\AppData\Local\Programs\Python\Python36\lib\multiprocessing\popen_spawn_win32.py", line 65, in __init__ reduction.dump(process_obj, to_child) File "C:\Users\j_maa\AppData\Local\Programs\Python\Python36\lib\multiprocessing\reduction.py", line 60, in dump ForkingPickler(file, protocol).dump(obj) ValueError: ctypes objects containing pointers cannot be pickled Traceback (most recent call last): File "<string>", line 1, in <module> File "C:\Users\j_maa\AppData\Local\Programs\Python\Python36\lib\multiprocessing\spawn.py", line 99, in spawn_main new_handle = reduction.steal_handle(parent_pid, pipe_handle) File "C:\Users\j_maa\AppData\Local\Programs\Python\Python36\lib\multiprocessing\reduction.py", line 87, in steal_handle _winapi.DUPLICATE_SAME_ACCESS | _winapi.DUPLICATE_CLOSE_SOURCE) PermissionError: [WinError 5] Toegang geweigerd

vincentbernat commented 6 years ago

OK, could you try without the multiprocessing module (revert the second commit, git revert d99dbd8)?

DhrBaksteen commented 6 years ago

That way it works, but playback is too slow.

vincentbernat commented 6 years ago

Which of the "late" counters is increasing?

DhrBaksteen commented 6 years ago

Transmit remains at 0, but CPU increases.

vincentbernat commented 6 years ago

Maybe serial transmission on Windows is done synchronously by default (on Linux, write is done to a 4K buffer and executed asynchronously). In pyserial, there is a write_timeout setting that could be set to 0. In this case, write() needs to be wrapped to capture the timeout exception we may get. http://pyserial.readthedocs.io/en/latest/pyserial_api.html

vincentbernat commented 4 years ago

This has been superseded by SerialPassthrough.ino.