rtrussell / BBCSDL

BBC BASIC for SDL 2.0: for Windows, Linux (86), MacOS, Raspberry Pi, Android and iOS.
zlib License
213 stars 28 forks source link

Testing for characters available on a serial connection not working #35

Closed Memotech-Bill closed 1 year ago

Memotech-Bill commented 1 year ago

In order to implement file upload to a Pico, I need to be able to test for characters received from the Pico.

From the documentation, I should be able to do this using the EXT# statement. However I have code of the form:

  devname$ = "/dev/ttyACM0."
  sd% = OPENUP(devname$)

  IF EXT#sd% > 0 THEN
    ch% = BGET#sd%

The code is hanging on the BGET# statement.

On using ESC to terminate the program, then doing PRINT EXT#sd% I get a value of 3988. It is extremely unlikely that there are that many characters in the buffer.

This is using the latest version of BBCSDL running on a RPi 4B with 32-bit Raspberry Pi OS, The complete code is attached. yupload.zip

rtrussell commented 1 year ago

doing PRINT EXT#sd% I get a value of 3988. It is extremely unlikely that there are that many characters in the buffer.

This is the code which I use for EXT# on a non-file device:

    int waiting = 0 ;
    ioctl (fileno (file->hidden.stdio.fp), FIONREAD, &waiting) ;
    return waiting ;

Because it is accessing a private (hidden) member of an SDL2 structure it's possible that they have changed the definition of that member and it no longer contains the FILE* value needed. If that is the cause I don't know how to proceed.

rtrussell commented 1 year ago

it's possible that they have changed the definition of that member and it no longer contains the FILE* value needed.

That doesn't seem to be the case. The most recent SDL source (which is actually SDL 3.0) still seems to use that member in the same way. So I don't understand why EXT# is returning an incorrect value.

What value is OPENUP returning in sd%?

Memotech-Bill commented 1 year ago

OPENUP is setting sd% to 1.

I have put some diagnostic writes in the code for getext(). The value of fileno is being returned as 9, which also looks reasonable. The man page for fileno() says that it should return -1 if the stream is not valid.

Sometimes the value of waiting is zero or another reasonable number. On other occasions it is a large value. I also seem to be reading a lot of NULL bytes. I really need to work out some way of double checking what the Pico is sending.

As a side issue, I realised that BPUT# was not transmitting single characters. I needed some way of flushing the serial output. To achieve this I have modified setptr() in bbcmos.c:

// Set file pointer:
void setptr (void *chan, long long ptr)
{
    if (chan <= (void *)MAX_PORTS)
        {
        // Flush device buffer
        SDL_RWops *file = lookup (chan) ;
        fflush (file->hidden.stdio.fp);
        return;
        }
    if ((chan > (void *)MAX_PORTS) && (chan <= (void *)(MAX_PORTS+MAX_FILES)))
        {
        int index = (size_t) chan - MAX_PORTS - 1 ;
        unsigned char *buffer = (unsigned char *) filbuf[0] + index * 0x100 ;
        FCB *pfcb = &fcbtab[index] ;
        SDL_RWops *handle = (SDL_RWops *) filbuf[(size_t) chan] ;
        if (writeb (handle, buffer, pfcb))
            error (189, SDL_GetError ()) ;
        *(int *)pfcb = 0 ;
        }
    if (-1 == SDL_RWseek (lookup (chan), ptr, RW_SEEK_SET))
        error (189, SDL_GetError ()) ;
}
rtrussell commented 1 year ago

As a side issue, I realised that BPUT# was not transmitting single characters

Why do you ever need to forcefully flush the buffer? The contents should always be sent 'in a timely fashion', which ought to mean a delay of a few milliseconds at most. It shouldn't buffer that 'single character' indefinitely.

We need to be ultra careful (and I cannot emphasise this too strongly) not to add something to one version of BBC BASIC which does not work the same way in other versions of BBC BASIC (notably, but not limited to, Matrix Brandy BASIC, BBC BASIC for Windows, the Console Mode editions and the editions of BBC BASIC for SDL 2.0 that are coded in assembly language rather than C).

You haven't even made the fflush() conditional on __LINUX__ being defined, nor added an equivalent routine for __WIN32__, so it's not a usable solution currently. It also won't work if SDL itself buffers the output (which it does in Windows, at least) because you are flushing at the Linux level and you have no way of knowing whether SDL has actually written out that byte yet.

There is an outstanding request to add an SDL_RWflush() function, which would be the proper way to do it, but it's probable that it will be implemented only in SDL3, not SDL2. Maybe when it is we could steal the code and put it in BBC BASIC.

There are a couple of things you could try as an alternative, and cross-platform, method of encouraging the byte to be sent. One is to add a WAIT 0, which will call sleep() behind the scenes; I would expect that to perform any deferred operations such as lazy writes. The other, more drastic, approach would be to close-and-reopen the port, but that might have some undesirable side-effects.

rtrussell commented 1 year ago

It also won't work if SDL itself buffers the output (which it does in Windows, at least)

Actually, looking at the code, it may be that it's only reads (and not writes) that are buffered in Windows, but nevertheless it's an assumption that one should ideally not make since there's no contractual guarantee. It's a somewhat strange asymmetry.

Memotech-Bill commented 1 year ago

I now believe that my initial error report was wrong. EXT# may be working as documented, I was confused by other issues.

However I don't believe it is possible to write a Pico upload program using BBCSDL. Issues I have encountered are:

It would be possible to develop a custom file transfer protocol which only uses printable characters, and each transaction ends in CR. However the VT100 cursor query would still cause problems initiating the transfer.

For USB connections, others have implemented two serial ports over the single USB connection. It might be possible to create a second serial connection for uploading which does not have the VT100 escape sequences. But that would not solve the problem for anyone connecting over UART.

I suppose it would be possible to use the assembler to provide an unbuffered connection to the Pico, but that would require a different version for every platform.

rtrussell commented 1 year ago
  • Received data buffering means that BGET# does not see any received characters until after a CR character is received.
  • The VT100 cursor position query (ESC [6n) being sent by the Pico seems to be being echoed back to the Pico when BBCSDL connects to the serial device.
  • Sending the first YModem block (which contains SOH(0x01) and NUL(0x00) bytes) results in BGET# just returning NUL bytes

None of these issues would seem, from your description, to have anything to do with BBC BASIC. Clearly neither BPUT# nor BGET# could be doing anything like that, since they couldn't then be used with binary files or binary data sent down a serial link. I would guess the cause is that your device driver is mis-configured, perhaps it is in some sort of 'cooked' rather than 'raw' mode. Surely that can be resolved by using a tcsetattr() call or similar?

rtrussell commented 1 year ago

Surely that can be resolved by using a tcsetattr() call or similar?

Looking back at some of my early tests with USB serial ports, my setup code included this:

OSCLI "stty -F /dev/ttyUSB0 9600 raw;"

If you are using something similar, did you remember to add the raw?

Memotech-Bill commented 1 year ago

How do you obtain the operating system file number from the BBC BASIC channel number? How do you call tcsetattr? Is it available via SYS? Does this work for all versions of BBC BASIC?

Calling an external program is not exactly doing it in BBC BASIC. Also it is specific to Linux, it would need a different program in Windows and perhaps also in IOS. And the SDL buffering may still get in the way. Still I will have to experiment with this.

rtrussell commented 1 year ago

Calling an external program is not exactly doing it in BBC BASIC.

Sorry, I'm afraid I don't understand the point you are making. The reason you are getting the corrupted data is specifically to do with the USB device/driver you are using, so it's hardly surprising that the solution will also be specific to that device and not generic BBC BASIC code.

BBC BASIC is not to know that when you specify /dev/ttyACM0. in your program that this is a particular kind of device that requires configuring to raw mode (if that is indeed the issue) to work with your program. It knows it's a device rather than a file (so for example it uses unbuffered input, whereas it buffers a file), but that's all it knows.

Also it is specific to Linux, it would need a different program in Windows and perhaps also in IOS.

But /dev/ttyACM0. is also "specific to Linux", and that's hard-coded in your program! If you choose to hard-code the device name, also hard-coding the configuration of that device seems perfectly reasonable to me. Whether that's done by 'shelling out' to a command-line tool like stty or using SYS to call a function in a shared library is a matter of personal preference.

Coming from a Windows background I prefer calling a function in a DLL, but somebody more familiar with Linux might be happier with the command-line tool.

Memotech-Bill commented 1 year ago

But /dev/ttyACM0. is also "specific to Linux", and that's hard-coded in your program!

"/dev/ttyACM0" is not hard-coded into the program. The user is prompted to supply a device name (which may include connection details such as baud rate), and this is stored in @usr$ + "pico.cfg".

Coming from a Windows background I prefer calling a function in a DLL.

I am not sure whether tcsetattr is in a DLL. It may be a direct operating system trap. If it is in a DLL I don't know which one. Does BBCSDL have the ability to load arbitrary Linux DLLs?

To be able to call tcsetattr I would still need to know the operating system file number.

rtrussell commented 1 year ago

"/dev/ttyACM0" is not hard-coded into the program. The user is prompted to supply a device name (which may include connection details such as baud rate), and this is stored in @usr$ + "pico.cfg".

Given that the way the baud rate is set is device and platform-specific, does that file contain the entire command for setting it (e.g. an stty command)? If so it can presumably contain the raw setting too, if that's all that's needed to fix the problems you were encountering.

I am not sure whether tcsetattr is in a DLL.

DLL is a term specific to Windows, as far as I know; in Linux it would be a .so file I assume.

It may be a direct operating system trap.

This is where my inexperience with Linux comes in, I don't know what that is! In Windows everything is done with a DLL.

To be able to call tcsetattr I would still need to know the operating system file number.

You probably want the @hfile%() array.

Memotech-Bill commented 1 year ago

According to the documentation, the baud rate can be set in the string passed to OPENUP. That is what (in my current draft program) is configured by the user.

rtrussell commented 1 year ago

According to the documentation, the baud rate can be set in the string passed to OPENUP. That is what (in my current draft program) is configured by the user.

That's the BBC BASIC for Windows documentation; you're using BBC BASIC for SDL 2.0 (I presume) so you should be looking here. There isn't even a Serial and Parallel I/O section in the BBCSDL manual, because there's no feasible way to support that on a cross-platform basis.

It's both the strength and weakness of SDL 2.0: If SDL2 supports it, so does BBCSDL (across all the supported platforms); if SDL2 doesn't support it, neither does BBCSDL. There are a lot of things I would like SDL2 to support which it currently doesn't (output to a hardcopy device is one), and judging by the comments at the SDL forum many people feel the same way. But we are all completely at their mercy.

The sole extent to which BBCSDL has any 'device-related' functionality is what we have already discussed: if the string passed to OPENUP starts with XXX: (in Windows) or /dev (on all other platforms) it uses unbuffered I/O and implements EXT# in an appropriate way for the platform. That's it.

Device and platform-specific setup, such as baud-rate and other serial parameters, must be performed independently (typically using OSCLI "stty..." in Linux and SYS "SetCommState", ... in Windows). Good luck trying to do it in MacOS, Android or iOS, I wouldn't know where to start!

Memotech-Bill commented 1 year ago

That's the BBC BASIC for Windows documentation; you're using BBC BASIC for SDL 2.0 (I presume) so you should be looking here. There isn't even a Serial and Parallel I/O section in the BBCSDL manual, because there's no feasible way to support that on a cross-platform basis.

The problem with that is that a large number of the links from the BBCSDL contents page point to pages in the BBC4W manual. I got to the serial port description from the SDL contents page in two steps (via the page for OPENUP).

rtrussell commented 1 year ago

The problem with that is that a large number of the links from the BBCSDL contents page point to pages in the BBC4W manual. I got to the serial port description from the SDL contents page in two steps (via the page for OPENUP).

I don't know how to avoid that. Inevitably much of the BBCSDL documentation is identical to the equivalent BB4W documentation, and it would be wasteful (and problematic from a source control viewpoint) to duplicate it. But that means if you end up at a BB4W manual page by following a link from the BBCSDL contents, the link to the contents now takes you to the BB4W contents not to the BBCSDL contents!

If it is possible to avoid that using 'standard' HTML, which I'm somewhat doubtful of, please let me know (I'm guessing it would mean some sort of symbolic way to represent the contents page rather than a simple relative or absolute URL). There's no point listing (e.g.) some JavaScript to do it, if you are tempted to do that, because I don't have the first idea how to incorporate that (I'm afraid my web-coding 'skills' are barely at the HTML 4.0 Transitional stage!).

Memotech-Bill commented 1 year ago

I agree that it is difficult. I think the best you can do is always make clear what is BB4W specific or BBCSDL specific.

Perhaps adopt some form of colour coding and have either the text or the background in some colour indicating that a particular section of description is specific to a particular version.

Memotech-Bill commented 1 year ago

To be able to call tcsetattr I would still need to know the operating system file number.

You probably want the @hfile%() array.

With the line:

PRINT "sd% = ";sd%;" @hfile%(sd%) = "; @hfile%(sd%)

I am getting:

sd% = 1 @hfile%(sd%) = -1.3966863E9

Clearly @hfile%() is not correct.

rtrussell commented 1 year ago

Clearly @hfile%() is not correct.

The OS 'file number' is a pointer (a HANDLE in BBC BASIC for Windows and a SDL_RWops* in BBC BASIC for SDL 2.0), and a pointer can legitimately be almost anything. How can you conclude, without further checks, that it's "not correct"?

Since I believe the original "Testing for characters available on a serial connection not working" issue has been resolved, and the subsequent discussion has wandered way off-topic, I am going to close the issue now.