robhagemans / pcbasic

PC-BASIC - A free, cross-platform emulator for the GW-BASIC family of interpreters
http://www.pc-basic.org
Other
396 stars 48 forks source link

crash on serial port open in Python 3 #125

Closed incanus closed 3 years ago

incanus commented 3 years ago

Bug report

Problem

With Python 3 and pySerial 3.4, OPEN crashes due to bytes passed instead of a string for the parity argument. In Python 2 with the same pySerial, it works fine since this is passed directly as a string.

Steps

  1. Launch PC-BASIC with (e.g. on Mac) com1=PORT:/dev/tty.usbserial-0001
  2. OPEN "COM1:9600,N,8,1,,CS,DS" as #1
  3. Crash

Crash log

PC-BASIC crash log
====================================================================================================
FATAL ERROR
version   2.0.3 [v2.0.2-1-820-g6807f35bb 2020-09-26 16:10:49.837131]
python    3.8.6 [64bit ] 
platform  macOS-10.16-x86_64-i386-64bit
interface VideoSDL2, AudioPlugin
statement :OPEN "com1:9600,n,8,1,,cs,ds" AS #1

files.py:94, open
ports.py:70, open
ports.py:261, set_params
serialutil.py:332, parity
ValueError: Not a valid parity: b'N'

This is a bug in PC-BASIC.
Sorry about that. You can help improve PC-BASIC:

- Please file a bug report, including this message and the steps you took
  just before the crash. Go to:
    https://github.com/robhagemans/pcbasic/issues

- Please include the full crash log in your report.
  You can paste it from the clipboard or from the file at:
    /Users/incanus/Library/Application Support/pcbasic-2.0/crash-20201103-qtkmlqh7.log
Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/pcbasic/guard.py", line 51, in protect
    yield
  File "/usr/local/lib/python3.8/site-packages/pcbasic/main.py", line 119, in _run_session
    session.interact()
  File "/usr/local/lib/python3.8/site-packages/pcbasic/basic/api.py", line 149, in interact
    self._impl.interact()
  File "/usr/local/lib/python3.8/site-packages/pcbasic/basic/implementation.py", line 315, in interact
    self.interpreter.loop()
  File "/usr/local/lib/python3.8/site-packages/pcbasic/basic/interpreter.py", line 122, in loop
    self.parse()
  File "/usr/local/lib/python3.8/site-packages/pcbasic/basic/interpreter.py", line 112, in parse
    self.parser.parse_statement(ins)
  File "/usr/local/lib/python3.8/site-packages/pcbasic/basic/parser/statements.py", line 82, in parse_statement
    self._callbacks[c](parse_args(ins))
  File "/usr/local/lib/python3.8/site-packages/pcbasic/basic/devices/files.py", line 329, in open_
    self.open(number, name, b'D', mode, access, lock, reclen)
  File "/usr/local/lib/python3.8/site-packages/pcbasic/basic/devices/files.py", line 94, in open
    new_file = device.open(
  File "/usr/local/lib/python3.8/site-packages/pcbasic/basic/devices/ports.py", line 70, in open
    self.set_params(speed, parity, bytesize, stop)
  File "/usr/local/lib/python3.8/site-packages/pcbasic/basic/devices/ports.py", line 261, in set_params
    self._serial.parity = parity
  File "/usr/local/lib/python3.8/site-packages/serial/serialutil.py", line 332, in parity
    raise ValueError("Not a valid parity: {!r}".format(parity))
ValueError: Not a valid parity: b'N'

Notes

A simple fix which seems to work in both Python 2 & 3 is amending pcbasic/basic/devices/ports.py as follows:

diff --git a/pcbasic/basic/devices/ports.py b/pcbasic/basic/devices/ports.py
index 6dd5d75a..690db968 100644
--- a/pcbasic/basic/devices/ports.py
+++ b/pcbasic/basic/devices/ports.py
@@ -258,7 +258,7 @@ class COMDevice(Device):
         with safe_io(error.DEVICE_FAULT):
             self._check_open()
             self._serial.baudrate = speed
-            self._serial.parity = parity
+            self._serial.parity = parity.decode('ascii')
             self._serial.bytesize = bytesize
             self._serial.stopbits = stop

All of the other arguments in that section are numeric and don't experience the problem.

robhagemans commented 3 years ago

Thanks! I think your solution is the correct one, I'll have a look. Thanks for testing this, unfortunately I don't have serial hardware so I depend on shims and careful reading of the code

robhagemans commented 3 years ago

I've updated this in develop branch, should be included in next release.