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

Build missing network interface support routines #26

Open
Memotech-Bill opened this issue Jul 3, 2024 · 88 comments
Open

Build missing network interface support routines #26

Memotech-Bill opened this issue Jul 3, 2024 · 88 comments

Comments

@Memotech-Bill
Copy link
Owner

As per topic on Raspberry Pi forums, Richard Russel's build of PicoBB is missing network support routines.

>PRINT SYS "net_freeall"
         0

However my build has them:

>PRINT SYS "net_freeall"
 268532725

My build sequence is simply:

cd PicoBB/console/pico_w
make

We need to identify what is different about the way Richard builds the program.

@Memotech-Bill
Copy link
Owner Author

Looking at the signon messages, Richard's build has:

BBC BASIC for Pico Console v0.46, Build Jul 2 2024, USB Console, UART Console, Flash Filesystem, cyw43=gpio, SDL Sound, /dev/uart, Stack Check 4, RTC

Whereas my build has:

BBC BASIC for Pico W Console v0.46, Build Jul 3 2024, USB Console, UART Console, Flash Filesystem, cyw43=background, SDL Sound, /dev/uart, Min Stack, Stack Check 4, RTC

From this, I guess that Richard is building from the console/pico folder not the console/pico_w folder. Specifying BOARD=pico+w is not sufficient to enable WiFi support.

@rtrussell
Copy link
Contributor

rtrussell commented Jul 3, 2024

From this, I guess that Richard is building from the console/pico folder not the console/pico_w folder.

That's correct. My understanding has always been that the various build options are achieved by specifying them in the make command line. Indeed, referring back to the issue where this was previously discussed, these were the instructions you gave for building a Pico W version:

Then build the experimental version:

cd console/pico
make BOARD=pico_w CYW43=Y

Apart from the change to BOARD=pico+w that's what I am still doing.

@Memotech-Bill
Copy link
Owner Author

That's correct. My understanding has always been that the various build options are achieved by specifying them in the make command line. Indeed, referring back to the issue where this was previously discussed, these were the instructions you gave for building a Pico W version:

That is correct in principle. Doing make in the different folders just selects different default values.

However, as noted later in that thread:

* The CYW43 switch now supports a number of options to select the different SDK libraries:
  
  * CYW43=GPIO - Only support the GPIO functions (the onboard LED and power options). This version has the same amount of BASIC memory as the standard Pico build. It could become the default build, replacing the version for the standard Pico.
  * CYW43=POLL Use the cyw43_arch_poll library. Note that this does not eliminate callback functions. The callbacks are triggered when you call cyw43_arch_poll(), rather thai in response to a timer interrupt.
  * CYW43=BACKGROUND. Use my fixed version of the cyw43_arch_threadsafe_background library. I believe that this is the most convenient version to use with BASIC.

Looking at the CMakeLists.txt file, CYW43=Y should be equivalent to CYW43=BACKGROUND, so I am not quite sure why your build does not include network support. Are you still doing git checkout lwip_raw_tcp? That was an experimental branch for initial development of networking, and does not contain all the latest revisions.

Building in the console/pico_w folder was documented at the end of that thread.

@rtrussell
Copy link
Contributor

However, as noted later in that thread:

Hmm, "later in the thread" is only useful if there was a reason for me to read it, and there probably wasn't!

Are you still doing git checkout lwip_raw_tcp?

I don't think so..

Most of my BBCSDL builds are as 'idiot proof' as I can make them (using a bog-standard makefile). Unfortunately there are two that aren't - Android and Pico - because in each case I'm reliant on a build process that I'm not familiar with: Android Studio in the first case and CMake in the second.

So in those cases I just blindly follow instructions that I've written down from a previous occasion and seem to work. I don't know why I'm doing what I'm doing, and unless some error message or obvious malfunction results, I am none the wiser.

To cut to the chase, what should I be doing to build the most 'universal' version, with support for the Pico W but with the minimum degree of impact (e.g. in respect of available memory) when it's running on a non-W Pico?

@Memotech-Bill
Copy link
Owner Author

To cut to the chase, what should I be doing to build the most 'universal' version, with support for the Pico W but with the minimum degree of impact (e.g. in respect of available memory) when it's running on a non-W Pico?

That depends upon whether or not you want network support. For a version that will run on either Pico or Pico W, but without network support:

cd PicoBB
git pull --recurse-submodules
git checkout master
cd console/pico
make BBC_SRC=<path to your source>

For a version that will run on both Pico and Pico W, and has network support on the Pico W, at the cost of reduced memory available to BASIC on either host, replace cd console/pico with cd console/pico_w.

A few words of explanation:

git pull will download from GitHub any changes I have made to my repository. The --recurse-submodules means that any changes to your repository, or the LittleFS repository, will also be downloaded.

get checkout master ensures that you are on the master branch. You probably will be anyway, this is just a precaution.

make builds the program. The option BBC_SRC=<path to your source> is only necessary if you have a local copy of your source that contains changes that you have not yet committed to GitHub.

As a check, after building the program, you can use the command:

picotool info <filename.uf2>

to show the signon message without having to load the program into a Pico. The significant part is the cyw43=... string. cyw43=gpio indicates support for the Pico W LED, while cyw43=background indicates network support (as well as the Pico W LED).

@rtrussell
Copy link
Contributor

rtrussell commented Jul 4, 2024

That depends upon whether or not you want network support.

I did say 'universal' version, so naturally that means it needs to support the key feature of the Pico W, i.e. WiFi (which I assume is what the W stands for). Other differences between the non-W and W versions are largely incidental as far as I'm concerned, and I would expect that they can be handled within, for example, a GPIO library.

How far did you get in making the memory costs of networking at least partly conditional on actually using the socklib library? The sort of thing I would expect is that calling PROC_initsockets would rejig the memory map, moving HIMEM down to make space for the networking buffers, before enabling the 'background' functionality.

Ideally (but less important, because one can always restart the Pico) calling PROC_exitsockets should disable the background functionality and then restore the memory map. But if that's not practical or too difficult so be it.

cd PicoBB
git pull --recurse-submodules
git checkout master
cd console/pico
make BBC_SRC=

That's similar to what I do, except that I think I use git clone rather than git pull, I don't know if that is significant.
I don't use the BBC_SRC= option because at one point it didn't work properly, so my build includes a step in which I replace the source files in your cloned repository with mine. It may well be that I could use that option now.

I also explicitly replace the example programs and libraries with my versions, because commonly I will have made recent changes that aren't reflected in your repository.

@Memotech-Bill
Copy link
Owner Author

That depends upon whether or not you want network support.

I did say 'universal' version, so naturally that means it needs to support the key feature of the Pico W, i.e. WiFi (which I assume is what the W stands for). Other differences between the non-W and W versions are largely incidental as far as I'm concerned, and I would expect that they can be handled within, for example, a GPIO library.

The LED on the Pico W is controlled by a GPIO on the CYW43 chip, not a GPIO on the RP2040. It requires a different set of SDK routines to control this. If these are not present, no BBC BASIC library could control the Pico W LED.

How far did you get in making the memory costs of networking at least partly conditional on actually using the socklib library? The sort of thing I would expect is that calling PROC_initsockets would rejig the memory map, moving HIMEM down to make space for the networking buffers, before enabling the 'background' functionality.

All such memory allocation is deep within the lwip library. It is not obvious how one would donate some memory from that available to BBC BASIC.

cd PicoBB
git pull --recurse-submodules
git checkout master
cd console/pico
make BBC_SRC=

That's similar to what I do, except that I think I use git clone rather than git pull, I don't know if that is significant. I don't use the BBC_SRC= option because at one point it didn't work properly, so my build includes a step in which I replace the source files in your cloned repository with mine. It may well be that I could use that option now.

git clone downloads a copy of the entire repository, whereas git pull only downloads any changes sice the previous clone or pull.

@rtrussell
Copy link
Contributor

The LED on the Pico W is controlled by a GPIO on the CYW43 chip, not a GPIO on the RP2040. It requires a different set of SDK routines to control this. If these are not present, no BBC BASIC library could control the Pico W LED.

Is that not very similar to the difference between the GPIO on the RPi 1-4 and the GPIO on the RPi 5? On the earlier models the GPIO is driven by the main SoC chip, but on the RPi 5 it's driven by a separate chip which communicates with the SoC using the I²C bus. The software interfaces are completely different, but I don't need to build different versions of BBC BASIC for the different models, it's entirely handled within the gpiolib.bbc library.

If you're saying that it's impossible for a single UF2 to be able to drive the GPIO on both the original Pico and the Pico W, I don't understand why that would be the case. Surely a UF2 could contain the drivers for both and a (notional) GPIO library use whichever interface is appropriate for the model on which it is running, similar to how it works on the Raspberry Pi itself?

If it really is impossible, it makes the whole idea of a 'universal' UF2 moot.

All such memory allocation is deep within the lwip library. It is not obvious how one would donate some memory from that available to BBC BASIC.

I thought that the CPU in the Pico was a relatively simple 'microcontroller' type device, lacking the kind of hardware memory-management which could enforce such partitioning. In its absence, I don't see how anything in the system could 'know' that BASIC is sharing the 'network' memory, so long as any background processes also accessing that memory are disabled.

If you examine the memory map of the WiFi-enabled version, is the memory allocated to networking contiguous with the memory that BASIC uses? If it is, or it can be persuaded to be (e.g. by controlling the order in which various modules are initialised) can't you disable the background processes and simply allow BASIC to extend its memory into that region. How could anything know you'd done that?

@Memotech-Bill
Copy link
Owner Author

Memotech-Bill commented Jul 5, 2024

The LED on the Pico W is controlled by a GPIO on the CYW43 chip, not a GPIO on the RP2040. It requires a different set of SDK routines to control this. If these are not present, no BBC BASIC library could control the Pico W LED.

Is that not very similar to the difference between the GPIO on the RPi 1-4 and the GPIO on the RPi 5? On the earlier models the GPIO is driven by the main SoC chip, but on the RPi 5 it's driven by a separate chip which communicates with the SoC using the I²C bus. The software interfaces are completely different, but I don't need to build different versions of BBC BASIC for the different models, it's entirely handled within the gpiolib.bbc library.

If you're saying that it's impossible for a single UF2 to be able to drive the GPIO on both the original Pico and the Pico W, I don't understand why that would be the case. Surely a UF2 could contain the drivers for both and a (notional) GPIO library use whichever interface is appropriate for the model on which it is running, similar to how it works on the Raspberry Pi itself?

I think we are talking at cross purposes. On the Pico, the LED is controlled by one of the RP2040 GPIO's. On the Pico W, that GPIO is repurposed to control the CYW43 chip. The LED is then controlled by one of the extra GPIOs on the CYW43 chip, which do not exist on the Pico. The option CYW43=GPIO (or CYW43=BACKGROUND) selects including the drivers for the CYW43 GPIO, necessary to control these additional GPIO, in order to provide a "Universal" UF2.

All such memory allocation is deep within the lwip library. It is not obvious how one would donate some memory from that available to BBC BASIC.

I thought that the CPU in the Pico was a relatively simple 'microcontroller' type device, lacking the kind of hardware memory-management which could enforce such partitioning. In its absence, I don't see how anything in the system could 'know' that BASIC is sharing the 'network' memory, so long as any background processes also accessing that memory are disabled.

If you examine the memory map of the WiFi-enabled version, is the memory allocated to networking contiguous with the memory that BASIC uses? If it is, or it can be persuaded to be (e.g. by controlling the order in which various modules are initialised) can't you disable the background processes and simply allow BASIC to extend its memory into that region. How could anything know you'd done that?

Moving HIMEM down makes space available right in the middle of the RAM used by BASIC, between the program and CPU stack.

I am not sure without looking whether the lwip network buffer allocation is static or dynamic.

If it is static, then the compiler will locate the buffers within the .bss region. Even if it could be arranged that the buffers are at the end of the region, and that this region was adjacent to the BASIC region, then in order to move this memory into or out of that available to BASIC, it would be necessary to dynamically adjust PAGE, not HIMEM. I don't think that can be done while a program is running.

If they are dynamically allocated, then they will be on the C heap. Either the heap is located below BASIC RAM, in which case the same issue as for static buffers arises, or the entire heap is located at HIMEM. Even then, moving HIMEM down would introduce space below the existing heap, but the heap grows upwards, not down. Also, the socklib library would be sitting between the original C heap and the freed space.

The only way I can see to make what you suggest work would be to modify the lwip code to use some form of custom memory allocation for the network buffers, making use of space either freed at HIMEM or perhaps allocated as BASIC variables. I am not currently inclined to undertake that.

@Memotech-Bill
Copy link
Owner Author

The current RAM memory map for the console/pico_w build is:

  • RAM(rwx) : ORIGIN = 0x20000000, LENGTH = 72k
    • .data
    • .bss
    • .heap
  • SCRATCH_Y(rwx) : ORIGIN = 0x20012000, LENGTH = 190k
    • Bottom of BASIC RAM
      • Directory paths
      • Command line
    • PAGE
      • BASIC program
    • LOMEM
      • BASIC heap
      • BASIC stack
    • HIGHMEM
      • BASIC libraries
      • Core 0 CPU stack
  • SCRATCH_X(rwx) : ORIGIN = 0x20041800, LENGTH = 2k
    • Core 1 CPU stack

@rtrussell
Copy link
Contributor

I think we are talking at cross purposes.

Let me try to clarify things with a couple of simple questions:

  1. Is it possible to create a 'universal' UF2 which contains code for driving both the Pico W GPIO (including the LED) and the non-W Pico GPIO (selected at runtime if necessary)?
  2. If that is possible, why make the inclusion of the Pico W driver code controllable by the CYW43=GPIO switch? Surely it's easier and less confusing to include it in every build as standard, rather than making it optional.

The only way I can see to make what you suggest work would be to modify the lwip code to use some form of custom memory allocation for the network buffers,

That would be the best way, certainly; other approaches are likely to involve some nasty assembly-language tricks to move the CPU's stack around without it noticing!

Obviously I've not seen the code itself, but I'm surprised that you don't think it's worth attempting. If we are still talking in terns of 40K of memory, which I think is what you originally suggested, that's a large amount to lose 'permanently' when it's only actually needed whilst socklib is in use.

@Memotech-Bill
Copy link
Owner Author

  1. Is it possible to create a 'universal' UF2 which contains code for driving both the Pico W GPIO (including the LED) and the non-W Pico GPIO (selected at runtime if necessary)?

Yes

  1. If that is possible, why make the inclusion of the Pico W driver code controllable by the CYW43=GPIO switch? Surely it's easier and less confusing to include it in every build as standard, rather than making it optional.

The makefiles specify CYW43=GPIO or CYW43=BACKGROUND (which includes GPIO) by default. It is necessary to explicitly specify CYW43=NONE to omit the driver.

Obviously I've not seen the code itself, but I'm surprised that you don't think it's worth attempting. If we are still talking in terns of 40K of memory, which I think is what you originally suggested, that's a large amount to lose 'permanently' when it's only actually needed whilst socklib is in use.

lwip is a large and complex library. It will take some research to even work out what and how memory is allocated. Depending on how the code is structured it may be necessary to fork the entire library in order to change the memory allocation, with subsequent support issues.

Also it is not clear that the network buffers are the main issue. It may be that much of the memory loss is due to the background processing of the CYW43 full driver.

I will do a bit more research.

@rtrussell
Copy link
Contributor

It is necessary to explicitly specify CYW43=NONE to omit the driver.

OK. But - just so I'm absolutely clear - is that only if built in the console/pico_w directory or is it also included by default in the console/pico directory? I still haven't entirely got my head around the reason for having two different directories rather than a simple build option to enable networking, like all the other options.

lwip is a large and complex library. It will take some research to even work out what and how memory is allocated.

I don't doubt it, but even a complex library may well allocate the majority of its static and dynamic buffers quite near the start, and thereafter only refer to them symbolically (e.g. as a pointer), so it might not be complicated.

Also it is not clear that the network buffers are the main issue. It may be that much of the memory loss is due to the background processing of the CYW43 full driver.

Indeed, but if the background processing is completely disabled except for when socklib is active, they can hopefully all be lumped together.

@Memotech-Bill
Copy link
Owner Author

It is necessary to explicitly specify CYW43=NONE to omit the driver.

OK. But - just so I'm absolutely clear - is that only if built in the console/pico_w directory or is it also included by default in the console/pico directory? I still haven't entirely got my head around the reason for having two different directories rather than a simple build option to enable networking, like all the other options.

I currently maintain four builds:

  • Console, No Network
  • Console, With Network
  • GUI, No Network
  • GUI, With Network

For me it is most convenient to have separate folders for these. I simply have to do make without any parameters in each of console/pico, console/pico_w, bin/pico and bin/pico_w.

The "No Network" builds are retained because they do have more BASIC memory available than the "With Network" builds.

If you look at the Makefile in each of these folders, they are quite simple, they simply set the default value for each of the required parameters and then invoke src/pico/Makefile.

@rtrussell
Copy link
Contributor

I currently maintain four builds:

Fair enough, but I only distribute a single 'as universal as possible' build for the Pico (just as I do for all the other platforms) and it's the build most people are going to come across - it seems to be what Paul Porcelijn is using.

The "No Network" builds are retained because they do have more BASIC memory available than the "With Network" builds.

Which of course is exactly what I am trying to avoid. It's not really viable to have to load a different UF2 into the Pico in order to run a BASIC program that uses network access and then revert back to the original UF2 in order to run a program that has a large memory usage.

In an ideal world it should be possible to write, for example, an application which initially accesses the network in order to download some data files and then performs some processing on that data which needs a lot of memory.

@Memotech-Bill
Copy link
Owner Author

I currently maintain four builds:

Fair enough, but I only distribute a single 'as universal as possible' build for the Pico (just as I do for all the other platforms) and it's the build most people are going to come across - it seems to be what Paul Porcelijn is using.

Which is currently the build in the console/pico_w folder. Although I have had support requests for the other builds.

The "No Network" builds are retained because they do have more BASIC memory available than the "With Network" builds.

Which of course is exactly what I am trying to avoid. It's not really viable to have to load a different UF2 into the Pico in order to run a BASIC program that uses network access and then revert back to the original UF2 in order to run a program that has a large memory usage.

I have identified in the memory map a fairly large static allocation associated with network buffers:

 COMMON         0x200059a8     0x98c7 CMakeFiles/bbcbasic.dir/home/pi/pico/pico-sdk/lib/lwip/src/core/memp.c.obj
                0x200059a8                memp_memory_RAW_PCB_base
                0x20005a1c                memp_memory_TCP_SEG_base
                0x20005c20                memp_memory_PBUF_POOL_base
                0x2000ebc4                memp_memory_PBUF_base
                0x2000ecc8                memp_memory_TCP_PCB_LISTEN_base
                0x2000edac                memp_memory_REASSDATA_base
                0x2000ee50                memp_memory_UDP_PCB_base
                0x2000eed4                memp_memory_TCP_PCB_base
                0x2000f20c                memp_memory_SYS_TIMEOUT_base

At present I have not worked out how to replace this.

Even if I do manage to modify the lwip code it is not clear where within the BASIC memory I can allocate storage,

  • socklib subroutines cannot change HIMEM. The library is on one side, and the BASIC stack on the other.
  • It might be possible to allocate above the loaded libraries. But there is not much space there unless HIMEM is lowered before loading the socklib library.
  • The other option is to allocate space on the BASIC heap. Will such allocations get mistaken as BASIC variables? Which commands removes data from the heap? CLEAR, NEW, any others?

@rtrussell
Copy link
Contributor

rtrussell commented Jul 5, 2024

  • socklib subroutines cannot change HIMEM.

Subroutines can change HIMEM but what they can't (easily and safely) do is change BASIC's stack pointer whilst it is in use!

But the key thing is easily and safely, libraries are allowed to (and the ones I write often do) break the normal rules of good coding practice, because they are 'black boxes' that end users shouldn't be fiddling with.

Here's a procedure that does succeed in lowering HIMEM, and BASIC's stack pointer, to free up some memory (here 10,000 bytes) which could be used for networking buffers:

      DEF PROCtest
      LOCAL b%%, d%%, s%%
      DIM s%% LOCAL TRUE, b%% LOCAL 10000, d%% LOCAL TRUE
      WHILE s%% < HIMEM
        ?d%% = ?s%%
        d%% += 1 : s%% += 1
      ENDWHILE
      HIMEM = d%%
      ENDPROC

Which commands removes data from the heap? CLEAR, NEW, any others?

CHAIN.

@Memotech-Bill
Copy link
Owner Author

Here's a procedure that does succeed in lowering HIMEM, and BASIC's stack pointer, to free up some memory (here 10,000 bytes) which could be used for networking buffers:

And suppose some intermediate subroutine has taken the address of a LOCAL variable?

@rtrussell
Copy link
Contributor

And suppose some intermediate subroutine has taken the address of a LOCAL variable?

Give me an example, I can't see how it could fail so long as that code is the first thing in the procedure.

@rtrussell
Copy link
Contributor

It could be useful to return the original value of HIMEM to the caller, for example so you can restore it later:

      DEF PROCtest(RETURN s%%)
      LOCAL b%%, d%%
      DIM s%% LOCAL TRUE, b%% LOCAL 10000, d%% LOCAL TRUE
      WHILE s%% < HIMEM
        ?d%% = ?s%%
        d%% += 1 : s%% += 1
      ENDWHILE
      HIMEM = d%%
      ENDPROC

In case it isn't obvious, the reserved memory for the networking buffer is at (the new value of) HIMEM, not at b%%.

@Memotech-Bill
Copy link
Owner Author

I have managed to find a way to reclaim 38KB of memory for BASIC if networking is available but not used.

In order to make it work, it requires the following code added to routine clear() in file bbmain.c in your repository:

#ifdef PICO
    pico_clear ();
#endif

With my code modification, buffers used for networking are allocated on the BASIC heap in a similar way to BASIC strings. Unused buffers are recycled, but the allocated space is not freed until an explicit or implicit CLEAR. As a consequence of this, CLEAR also closes all network connections.

Looking at the revised memory map it is not clear that any further memory can be reclaimed.

During development it was a nasty shock to find that the value of the BASIC stack pointer (esp) needed to check for available space was not valid in routines called back from LWIP. This was caused by the variable being declared as a register variable in BBC.h. This was eventually resolved by adding the compiler switches -ffixed-r10 -ffixed-r11 so that the LWIP code did not use these registers.

More testing is needed. Should you wish to test, then git clone or git pull the latest version of my repository, then in folder PicoBB/console/pico_w run the command make NET_HEAP=1.

@rtrussell
Copy link
Contributor

I have managed to find a way to reclaim 38KB of memory for BASIC if networking is available but not used.

Excellent, I was hopeful that should be possible.

With my code modification, buffers used for networking are allocated on the BASIC heap

Unfortunately I don't think that will work, because one needs to be able to free the memory used for the buffers by calling PROC_exitsockets, so that for example the rest of the program can use that memory for processing data that it has previously downloaded from the network. If the buffers are allocated 'midway' through the heap there's no way of freeing it other than using CLEAR, which will likely throw away the very data that has been downloaded!

The sort of program structure that ideally needs to be supported is as follows:

PROC_initsockets : REM Initialise network access, including allocating buffers
REM. Download data from the net, for example into an array created on the heap
PROC_exitsockets : REM Free network buffers and close down network access
REM. Process the downloaded data, if necessary using the freed buffer memory.

It's a classic problem when your only options are to allocate buffers either on the heap or the stack, which is why I suggested allocating them by lowering HIMEM (for example using the code I recently published, and now documented at the Wiki). Memory above HIMEM is also better protected against accidental corruption (it's only too easy to splat the heap in BBC BASIC) and it would avoid the necessity to change bbmain.c.

This does of course still impose the limitation that PROC_initsockets and PROC_exitsockets can only safely be called when the stack is empty, i.e. from the 'main program', but that's not an uncommon limitation for libraries.

Another potential advantage is that it might allow for sockets to be kept open during a CHAIN (which calls CLEAR), in exactly the same way as files are kept open during CHAIN. This can allow multiple modules to access an open file or socket by passing its handle to the CHAINed module in a static integer variable.

Hopefully it will be as easy to allocate the buffers by lowering HIMEM as on the heap,

@Memotech-Bill
Copy link
Owner Author

What is wrong with the following?

Program 1:
    Download data from network
    Write data to file
    Chain to program 2

Program 2:
    Read data from file
    Process data

Since CHAIN clears variables, all memory is available for the analysis in program 2.

The problem is that the network code actually performs multiple small allocations while reading and writing network sockets.

Admittedly, the standard Pico SDK implementation (when using background processing) reserves a fixed static pool of memory, with size determined by constants in lwipopts.h, and then allocates memory out of this pool. This has two disadvantages:

  • Most often more memory than is needed is allocated.
  • It imposes an artificial upper limit on the amount of memory available for the network.

If memory is allocated by moving HIMEM there are two options:

  • Allocate a fixed memory pool in PROCinitsockets. This has the disadvantages listed above.
  • Move HIMEM and the BASIC stack down a bit more every time the network code needs more memory. This is likely to be slow, and I still have reservations about the existence of pointers to variables on the stack.

For a LOCAL structure, what is stored on the BASIC stack?

  • The variable name block?
  • The structure format block?
  • The structure data block?

If either the format or data block, while these will be moved when the stack is moved, the pointers in the name block will not be updated with the new position.

@rtrussell
Copy link
Contributor

What is wrong with the following?

I wouldn't say there is anything "wrong" with it, it's just messy and not something that a BBC BASIC programmer would expect to be necessary.

Admittedly, the standard Pico SDK implementation (when using background processing) reserves a fixed static
pool of memory, with size determined by constants in lwipopts.h, and then allocates memory out of this
pool. This has two disadvantages...

But are we really in the business of trying to 'improve' on what the "standard Pico SDK implementation" does (which is presumably what currently happens in the Pico W build)?

  • Allocate a fixed memory pool in PROCinitsockets. This has the disadvantages listed above.

It has those disadvantages, but it also has the advantages I described. You may take a different view, but it seems to me that the advantages outweigh the disadvantages. For example it doesn't matter too much if the static allocation is a little bigger than the total of the dynamic ones, if it can be freed for other purposes by calling PROC_exitsockets.

  • Move HIMEM and the BASIC stack down a bit more every time the network code needs more memory.

I entirely agree that's not acceptable, not least because of all the stack copying involved. One would want to allocate it all in one go, preferably at a time when the stack isn't in use, which is likely to be the case when PROC_initsockets is called. This is commonly the code that would be used, near the beginning of the program:

INSTALL @lib$ + "socklib"
PROC_initsockets

which pretty much guarantees that the stack won't be in use at the time.

If you really can't do it any other way I'll live with the heap allocation, but I'm not prepared to change bbmain.c, not least because in no version of BBC BASIC, right back to the original in 1981, has CLEAR ever had any 'side effects'. It would be breaking new ground, which is just the sort of thing I'm keen to avoid.

Therefore it would have to be calling PROC_exitsockets, not CLEAR, that closes down the network and releases, but doesn't free, the buffers. This is really no more dangerous than any of the other existing libraries that will leave the system in an unstable state if the associated 'cleanup' function isn't called before the program is terminated.

I would just want to add, though, that the interpreter makes assumptions about when the heap can and can't grow. This is particularly the case when 'long strings' (and in the case of the Pico version that's not very long!) are stored temporarily in the unallocated part of the heap (that is, the pointer to free memory isn't raised). This would rule out your code allocating heap memory in an asynchronous callback.

@Memotech-Bill
Copy link
Owner Author

Your code PROCtest copies down the contents of the BASIC stack, but as far as I can see it does nothing to actually move the stack pointer to point to the new stack location.

I have tried doing this from within C code. Testing with lanchat which calls PROC_initsockets at the start, and with my version of PROC_initsockets beginning:

REM Initialise the BBCSDL Sockets interface
DEF PROC_initsockets(N%)
DEF PROC_initsockets
LOCAL chan%, err%
SYS "net_init"

I find that when net_init() is entered, himem and esp differ by 56 bytes, so even in the case where PROC_initsockets is called from the start of the main program there is a non-trivial BASIC stack.

In C routine net_init(), I can move himem, copy the BASIC stack to the new location and update esp. However in routine xeq(): case TSYS (in bbexec.c) the value of esp is remembered in a local variable on entry, and restored after return from my net_init() routine. So, by the time execution returns to BASIC, the stack pointer is pointing to the original copy of the stack, not the moved one.

The only way I can see of making this work is if the code for xeq(): case THIMEML was modified to relocate the BASIC stack whenever HIMEM is changed. Or make the variable oldesp in xeq () a global variable, rather than a local one, so that my net_init() code can set the value of esp that is eventually returned.

As a side issue, I think the C variable esp is confusingly / incorrectly typed in BBC.h. It is currently typed as heapptr *esr;, which implies double indirection, so that *esr points to the basic stack. I think it should be typed void *esr; or char *esr; since the value of esr is the pointer to the BASIC stack.

@rtrussell
Copy link
Contributor

Your code PROCtest copies down the contents of the BASIC stack, but as far as I can see it does
nothing to actually move the stack pointer to point to the new stack location.

"As far as you can see" is not far enough, because it works! The stack pointer is indeed moved to the new location, as can easily be demonstrated:

      PRINT "On entry:"
      PRINT "HIMEM = &"; STR$~HIMEM
      DIM s%% LOCAL -1
      PRINT "Stack pointer = &"; ~s%%
      PROCtest
      PRINT "On exit:"
      PRINT "HIMEM = &"; STR$~HIMEM
      DIM s%% LOCAL -1
      PRINT "Stack pointer = &"; ~s%%
      END

      DEF PROCtest
      LOCAL b%%, d%%, s%%
      DIM s%% LOCAL TRUE, b%% LOCAL 10000, d%% LOCAL TRUE
      WHILE s%% < HIMEM
        ?d%% = ?s%%
        d%% += 1 : s%% += 1
      ENDWHILE
      HIMEM = d%%
      ENDPROC

I have tried doing this from within C code.

Don't (you've already been caught by trying to manipulate the stack pointer external to the interpreter, which you are not supposed to do)! Do it in BASIC code in PROC_initsockets and then pass the new value of HIMEM into your C code using SYS, then everything will work correctly.

As a side issue, I think the C variable esp is confusingly / incorrectly typed in BBC.h.

Sorry if it's confusing, but it's correct. Declaring it as void* would break the pointer arithmetic, because sizeof(void) is 1 (in GCC and Clang) whereas sizeof(heapptr) is 4, which is what we want. Doing it this way ensures that the stack pointer is always correctly aligned, whereas declaring it as void* would make it possible to adjust the stack pointer by non-multiples of 4 bytes.

@Memotech-Bill
Copy link
Owner Author

I can only assume that it works because the stack space reserved by b%% LOCAL 10000 is not freed when the routine exits, even though it is defined as LOCAL.

In that case, how do I free that space in PROC_exitsockets and move the stack pointer back up?

@rtrussell
Copy link
Contributor

In that case, how do I free that space in PROC_exitsockets and move the stack pointer back up?

This is one way (it's possible there is a better way, but I haven't given it much thought). In an ideal world one would probably want to pass the 'amount to reserve' or 'amount to free' as parameters rather than as embedded constants (10000) as here:

      PRINT "On entry:"
      PRINT "HIMEM = &"; STR$~HIMEM
      DIM s%% LOCAL -1
      PRINT "Stack pointer = &"; ~s%%
      PROCtest
      PRINT "After PROCtest:"
      PRINT "HIMEM = &"; STR$~HIMEM
      DIM s%% LOCAL -1
      PRINT "Stack pointer = &"; ~s%%
      PROCfree
      PRINT "After PROCfree:"
      PRINT "HIMEM = &"; STR$~HIMEM
      DIM s%% LOCAL -1
      PRINT "Stack pointer = &"; ~s%%
      END

      DEF PROCtest
      LOCAL b%%, d%%, s%%
      DIM s%% LOCAL TRUE, b%% LOCAL 10000, d%% LOCAL TRUE
      WHILE s%% < HIMEM
        ?d%% = ?s%%
        d%% += 1 : s%% += 1
      ENDWHILE
      HIMEM = d%%
      ENDPROC

      DEF PROCfree
      LOCAL b%%, d%%, s%%
      DIM s%% LOCAL TRUE, b%% LOCAL 7, d%% LOCAL TRUE
      b%% = 10000 + s%% - d%% - 4
      s%%!-12 += b%% : d%% = s%% + b%%
      WHILE s%% < HIMEM
        ?d%% = ?s%%
        d%% += 1 : s%% += 1
      ENDWHILE
      HIMEM = d%%
      ENDPROC

@Memotech-Bill
Copy link
Owner Author

Thanks for that. My version is:

 1 WHILE TRUE
 2 INPUT "Space to allocate", space%
 3 PRINT "Space requested = &";~space%
 4 PRINT "On entry:"
 5 PRINT "HIMEM = &"; STR$~HIMEM
 6 DIM s%% LOCAL -1
 7 PRINT "Stack pointer = &"; ~s%%
 8 PROC__socklib_rsrv(space%)
 9 PRINT "After PROC__socklib_rsrv:"
10 PRINT "HIMEM = &"; STR$~HIMEM
11 DIM s%% LOCAL -1
12 PRINT "Stack pointer = &"; ~s%%
13 PROC__socklib_free(space%)
14 PRINT "After PROC__socklib_free:"
15 PRINT "HIMEM = &"; STR$~HIMEM
16 DIM s%% LOCAL -1
17 PRINT "Stack pointer = &"; ~s%%
18 ENDWHILE
19 END
20 
21 REM Move HIMEM and BASIC stack down to make room for network heap
22 DEF PROC__socklib_rsrv(size%)
23 LOCAL b%%, d%%, s%%
24 DIM s%% LOCAL TRUE, b%% LOCAL size%-&20, d%% LOCAL TRUE
25 WHILE s%% < HIMEM
26 ?d%% = ?s%%
27 d%% += 1 : s%% += 1
28 ENDWHILE
29 HIMEM = d%%
30 ENDPROC
31 
32 REM Move HIMEM and BASIC stack up recovering space used by network heap
33 DEF PROC__socklib_free(size%)
34 LOCAL b%%, d%%, s%%
35 DIM s%% LOCAL TRUE, b%% LOCAL 7, d%% LOCAL TRUE
36 b%% = size% + s%% - d%% - &20
37 s%%!-12 += b%% : d%% = s%% + b%%
38 WHILE s%% < HIMEM
39 ?d%% = ?s%%
40 d%% += 1 : s%% += 1
41 ENDWHILE
42 HIMEM = d%%
43 ENDPROC

And test results:

Space to allocate? 8192
Space requested = &2000
On entry:
HIMEM = &2002A800
Stack pointer = &2002A7F8
After PROC__socklib_rsrv:
HIMEM = &20028800
Stack pointer = &200287F8
After PROC__socklib_free:
HIMEM = &2002A800
Stack pointer = &2002A7F8
Space to allocate? 4096
Space requested = &1000
On entry:
HIMEM = &2002A800
Stack pointer = &2002A7F8
After PROC__socklib_rsrv:
HIMEM = &20029800
Stack pointer = &200297F8
After PROC__socklib_free:
HIMEM = &2002A800
Stack pointer = &2002A7F8
Space to allocate? 2048
Space requested = &800
On entry:
HIMEM = &2002A800
Stack pointer = &2002A7F8
After PROC__socklib_rsrv:
HIMEM = &2002A000
Stack pointer = &20029FF8
After PROC__socklib_free:
HIMEM = &2002A800
Stack pointer = &2002A7F8
Space to allocate? 1024
Space requested = &400
On entry:
HIMEM = &2002A800
Stack pointer = &2002A7F8
After PROC__socklib_rsrv:
HIMEM = &2002A400
Stack pointer = &2002A3F8
After PROC__socklib_free:
HIMEM = &2002A800
Stack pointer = &2002A7F8
Space to allocate? 512
Space requested = &200
On entry:
HIMEM = &2002A800
Stack pointer = &2002A7F8
After PROC__socklib_rsrv:
HIMEM = &2002A600
Stack pointer = &2002A5F8

R0 = 00000043  R8  = FFFFFFFF
R1 = 00000000  R9  = 00000000
R2 = 00000000  R10 = 2000AB7F
R3 = 00000000  R11 = 2002A560
R4 = 2002A788  R12 = 200413D8
R5 = 00000043  SP  = 200415F8
R6 = 00000001  LR  = 100110E1
R7 = 00000001  PC  = 1000EEF4
SG = 2002A640  PSP = 61000000

Hard fault at line 39

So clearly a minimum that can be reclaimed.

@rtrussell
Copy link
Contributor

With my latest commit, try:

Now I've reduced the size of the filesystem by replacing the large prague.wav file with a shorter audio clip, is there still a benefit in doing this?

The only residual issue I'm trying to solve (and it's almost certainly 'finger trouble' on my part) is that despite using --recurse-submodules I'm still not picking up the latest source files from the BBCSDL repository. Maybe it's because I've been doing git pull to fetch your recent changes without having to start from scratch. This git stuff confuses me!

@Memotech-Bill
Copy link
Owner Author

Now I've reduced the size of the filesystem by replacing the large prague.wav file with a shorter audio clip, is there still a benefit in doing this?

Only to provide the user with more space for their files.

The only residual issue I'm trying to solve (and it's almost certainly 'finger trouble' on my part) is that despite using --recurse-submodules I'm still not picking up the latest source files from the BBCSDL repository. Maybe it's because I've been doing git pull to fetch your recent changes without having to start from scratch. This git stuff confuses me!

I am not a git expert, but I believe a sub-module refers to a specific commit of the parent repository. It is done that way so that any breaking changes to the parent repository does not break any projects using the repository as a sub-module.

I believe that the solution is:

cd PicoBB/BBCSDL
git pull

to pull your latest source files.

Alternately use

make BBCSDL=/path/to/your/source

At some point I need to do a new pull of your source into my repository, and then commit the change so that the sub-module points to your latest source.

@rtrussell
Copy link
Contributor

I believe that the solution is:

cd PicoBB/BBCSDL
git pull

That sounds like a good solution, but unfortunately it doesn't work:

You are not currently on a branch.
Please specify which branch you want to merge with.
See git-pull(1) for details.

    git pull <remote> <branch>

Alternately use

make BBCSDL=/path/to/your/source

I know I can do that*, but it requires the up-to-date BBCSDL files to be on the same machine, which is something I cannot always guarantee (they won't be if I've committed a change to the BBCSDL repository but never built it on this machine).

*Actually it doesn't seem to work. Are you sure it redirects both the /src and the /include directories, because superficially it's not picking up the up-to-date header files.

@Memotech-Bill
Copy link
Owner Author

That sounds like a good solution, but unfortunately it doesn't work:

OK, I have pulled your latest repository into mine, then committed and pushed the change. I think that you now need to do:

git pull --recurse-submodules

Alternately use

make BBCSDL=/path/to/your/source

*Actually it doesn't seem to work. Are you sure it redirects both the /src and the /include directories, because superficially it's not picking up the up-to-date header files.

It may not redirect the include files. I will have to check.

@Memotech-Bill
Copy link
Owner Author

Alternately use

make BBCSDL=/path/to/your/source

*Actually it doesn't seem to work. Are you sure it redirects both the /src and the /include directories, because superficially it's not picking up the up-to-date header files.

It may not redirect the include files. I will have to check.

My error :(

The command should be:

make BBC_SRC=/path/to/your/source

And yes, it does redirect the include files as well as the C source.

@rtrussell
Copy link
Contributor

OK, I have pulled your latest repository into mine, then committed and pushed the change.

I'm sorry to report that doing this now builds an invalid filesystem_pwc.uf2 (at least, my Pico doesn't accept it):

git pull --recurse-submodules
cd console/pico_w
make clean
make

filesystem_pwc.uf2 is 2,097,152 bytes but when copied to the Pico it eventually times out and the LED doesn't start flashing. On running BBC BASIC the filesystem contains no files, only some empty directories dev/, lib/, tmp/ and user/

@Memotech-Bill
Copy link
Owner Author

This has been caused by preparation for supporting the Pico 2.

For the original Pico, there was only one UF2 family ID value (0xe48bff56). For the Pico 2 there are a number of possible ID values, see section 5.5.3 of the RP2350 datasheet.

I have modified uf2merge so that it (optionally) picks up the family ID from the first file to be merged, and applies it to the subsequent files. Hence the bbcbasic+filesystem_*.uf2 file has the correct family ID (matching that of bbcbasic_*.uf2). However, when building filesystem_*.uf2 (which could apply to Pico or Pico 2) it is not obvious which family ID to apply.

The fundamental problem is that (in general) it is not easy (in the Makefile) to go from board name to chip type. It is done in CMake.

I need to give this some more thought.

@Memotech-Bill
Copy link
Owner Author

A bit of a cludge: I have added (and pushed) an option to uf2conv to read a reference UF2 file and use the family ID from the reference file as the family ID for the file being created.

Using this, my Makefile now gives filesystem_*.uf2 the same family ID as bbcbasic_*.uf2.

@rtrussell
Copy link
Contributor

I have modified uf2merge so that it (optionally) picks up the family ID from the first file to be merged, and applies it to the subsequent files.

OK. For my release version I don't use your uf2merge, I use mine (written a long time ago) because it creates a significantly smaller file (or at least it used to, things may have changed for the pico_w). So I'll need to make the same modification to that, thanks for the heads-up.

@Memotech-Bill
Copy link
Owner Author

OK. For my release version I don't use your uf2merge, I use mine (written a long time ago) because it creates a significantly smaller file (or at least it used to, things may have changed for the pico_w).

That is because my version used to zero fill all the space between the executable and the filesystem. I have revised this recently.

@rtrussell
Copy link
Contributor

That is because my version used to zero fill all the space between the executable and the filesystem.

I guessed that, mine never has. I'll modify my version so it ignores the family ID in the filesystem file altogether.

@rtrussell
Copy link
Contributor

I'll modify my version so it ignores the family ID in the filesystem file altogether.

That seems to be working, both your merged file and mine now load.

Pretty much everything seems to be resolved now. I thought I'd try running mysqldem.bbc as in principle that should run if the socket interface is working, but it reports:

Error -7 connecting to access point at line 93

Is that anything I should worry about?

@Memotech-Bill
Copy link
Owner Author

Don't know.

When I try it, I get:

Connecting to MySQL server 'mysql-rfam-public.ebi.ac.uk'... failed to connect, aborting

Do any of the other network programs, such as lanchat work for you?

@rtrussell
Copy link
Contributor

When I try it, I get:
Connecting to MySQL server 'mysql-rfam-public.ebi.ac.uk'... failed to connect, aborting

Does the PicoW do DNS lookups, or do you have to supply an IP address? I notice that wifi-cfg.bbc doesn't ask for a DNS server (or even a netmask).

nslookup mysql-rfam-public.ebi.ac.uk
Server:  fritz.box
Address:  fd00::de15:c8ff:fe70:f7e9

Non-authoritative answer:
Name:    mysql-vm-084.ebi.ac.uk
Address:  193.62.194.222
Aliases:  mysql-rfam-public.ebi.ac.uk

Do any of the other network programs, such as lanchat work for you?

lanchat worked fine for me yesterday, that's how I concluded the PicoW had successfully connected to the network, but today I can't get the WiFi to work at all. Everything results in a hard fault, e.g.:

LittleFS image v2.1, Size = 1024KB, Origin = 0x10100000
BBC BASIC for Pico W Console v0.47, Build Oct 19 2024, USB Console, UART Console
, Flash Filesystem, cyw43=background, SDL Sound, /dev/uart, Min Stack, Stack Che
ck 4, RTC
(C) Copyright R. T. Russell, 2024
cyw43 initialisation succeded
>CH."wifi_scan"

R0 = 00000027  R8  = 20041698
R1 = 00000000  R9  = FFFFFFFD
R2 = 00000000  R10 = 00000000
R3 = 2000371C  R11 = 00000000
R4 = FFFFFFD8  R12 = 20005324
R5 = 00000020  SP  = 20041440
R6 = 200052A4  LR  = 1002E221
R7 = 200052A4  PC  = 1001828A
SG = 20028840  PSP = 01000000

Hard fault at line 70
>

I've no idea what has changed.

@Memotech-Bill
Copy link
Owner Author

Does the PicoW do DNS lookups, or do you have to supply an IP address? I notice that wifi-cfg.bbc doesn't ask for a DNS server (or even a netmask).

Yes it should do DNS lookups. It relies upon DHCP from the WiFi to provide netmask, gateway and DNS configuration.

@rtrussell
Copy link
Contributor

rtrussell commented Oct 20, 2024

Yes it should do DNS lookups. It relies upon DHCP from the WiFi to provide netmask, gateway and DNS configuration.

It should find mysql-rfam-public.ebi.ac.uk then, I've never had problems connecting to it from any other client.

Edit: Modifying mysqldem.bbc to use the explicit IP address 193.62.194.222 works about one time out of three here. The other times I get a Hard Fault or a Error -7 connecting to access point at line 93.

I see that wifi_scan.bbc (when it works) lists MAC addresses, which is useful here because we have a mesh, so there are multiple APs with the same SSID. Is there a way of forcing the PicoW to connect to a specific MAC address (hence mesh node)?

@Memotech-Bill
Copy link
Owner Author

The current version of wifi_scan is broken. It does not call PROC_initsockets so it does not reserve the space above HIMEM needed for the network routines.

Once you get a hard fault you will need to do a QUIT to re-initialise variables before any networking will work.

PROC_initsockets does too much, since it connects to an access point. I need to add a routine to socklib to just reserve the memory. This is not currently working properly, I need further investigation.

Is there a way of forcing the PicoW to connect to a specific MAC address (hence mesh node)?

PROC_initsockets includes:

SYS "cyw43_arch_wifi_connect_timeout_ms", ssid$, pwd$, &400004, 30000 TO err%

I see that the C SDK also lists:

int cyw43_arch_wifi_connect_bssid_timeout_ms (const char * ssid, const uint8_t * bssid, const char * pw, uint32_t auth, uint32_t timeout)

I assume that the BSSID is the MAC address.

It would be necessary to write some code to detect colons on the SSID, convert the string to bytes, and call the alternate connection routine.

@rtrussell
Copy link
Contributor

The current version of wifi_scan is broken.

Ah. I was about to release the latest build but it sounds like I should hold fire.

I need to add a routine to socklib to just reserve the memory.

These were the routines I originally suggested you use (I think you made a small change because of an off-by-one error when run on the Pico rather than in Windows). Of course the 64-bit pointers aren't necessary on a 32-bit platform:

      PRINT "On entry:"
      PRINT "HIMEM = &"; STR$~HIMEM
      DIM s%% LOCAL -1
      PRINT "Stack pointer = &"; ~s%%
      PROCtest
      PRINT "After PROCtest:"
      PRINT "HIMEM = &"; STR$~HIMEM
      DIM s%% LOCAL -1
      PRINT "Stack pointer = &"; ~s%%
      PROCfree
      PRINT "After PROCfree:"
      PRINT "HIMEM = &"; STR$~HIMEM
      DIM s%% LOCAL -1
      PRINT "Stack pointer = &"; ~s%%
      END

      DEF PROCtest
      LOCAL b%%, d%%, s%%
      DIM s%% LOCAL TRUE, b%% LOCAL 10000, d%% LOCAL TRUE
      WHILE s%% < HIMEM
        ?d%% = ?s%%
        d%% += 1 : s%% += 1
      ENDWHILE
      HIMEM = d%%
      ENDPROC

      DEF PROCfree
      LOCAL b%%, d%%, s%%
      DIM s%% LOCAL TRUE, b%% LOCAL 7, d%% LOCAL TRUE
      b%% = 10000 + s%% - d%% - 4
      s%%!-12 += b%% : d%% = s%% + b%%
      SYS "memmove", d%%, s%%, HIMEM - s%%
      HIMEM = d%% + HIMEM - s%%
      ENDPROC

It would be necessary to write some code to detect colons on the SSID, convert the string to bytes, and call the alternate connection routine.

OK. This is low priority, but could be useful as a future enhancement for those who have mesh systems.

@Memotech-Bill
Copy link
Owner Author

The issues with wifi_scan have now been resolved.

It took a while due to tracking down a memory corruption issue that was eventually traced to an out by one error in the sizes of a structure between C & BASIC.

@Memotech-Bill
Copy link
Owner Author

Yes it should do DNS lookups. It relies upon DHCP from the WiFi to provide netmask, gateway and DNS configuration.

It should find mysql-rfam-public.ebi.ac.uk then, I've never had problems connecting to it from any other client.

Edit: Modifying mysqldem.bbc to use the explicit IP address 193.62.194.222 works about one time out of three here. The other times I get a Hard Fault or a Error -7 connecting to access point at line 93.

I have fixed an issue with DNS lookup.

I am now failing to connect to the server due to a 10 second timeout.

@rtrussell
Copy link
Contributor

I am now failing to connect to the server due to a 10 second timeout.

When it works here (using the IP address) it gives the expected output:

image

@rtrussell
Copy link
Contributor

The issues with wifi_scan have now been resolved.

Thank you. I plan to release the latest version; it's still very prone to Hard Faults here, including in the circumstances described in the other issue, but perhaps my Pico W is faulty in some way.

@Memotech-Bill
Copy link
Owner Author

Memotech-Bill commented Oct 22, 2024

It appears that the LWIP DNS resolver is limited in its capability. It often fails to resolve mysql-rfam-public.ebi.ac.uk although it seems reliable at resolving bbc.co.uk or google.co.uk. I have pushed a simple test program console/pico_w/examples/dns_lookup.bas.

Replacing the domain name by IP address in mysqldem.bbc I obtain:

Connecting to MySQL server '193.62.194.222'... connected successfully

Connecting to database 'Rfam'... connected successfully
MySQL version = 5.6.36-log

Fetching database names... DIM space at line 314

Are you running a modified version of the program?

Edit:

After changing line 314 to:

DIM payload&(1024)

I also obtain:

Connecting to MySQL server '193.62.194.222'... connected successfully

Connecting to database 'Rfam'... connected successfully
MySQL version = 5.6.36-log

Fetching database names... 12 database(s) enumerated: 'information_schema' 'Rfam' 'mysql' 'performance_schema' 'rfam_12_2' 'rfam_13_0' 
'rfam_14_0' 'rfam_14_10' 'rfam_14_8' 'rfam_14_9' 'sj_test' 'sys'

Fetching table names... 14 table(s) enumerated: '_annotated_file' '_family_file' '_genome_data' '_lock' '_overlap' '_overlap_membership
' '_post_process' 'alignment_and_tree' 'author' 'clan' 'clan_database_link' 'clan_literature_reference' 'clan_membership' 'database_lin
k'

Querying database 'Rfam' for table 'author'...

Fetching column titles... number of columns = 6
Fetching table data... number of rows read = 14

author_id  name                  last_name             initials    orcid                 synonyms
1          Ames T                Ames                  T                                 
2          Argasinska J          Argasinska            J           0000-0003-2678-2824   
3          Bachellerie JP        Bachellerie           JP                                
4          Barquist LE           Barquist              LE          0000-0003-4732-2667   
5          Barrick JE            Barrick               JE                                
6          Bateman A             Bateman               A           0000-0002-6982-4660   
7          Boese B               Boese                 B                                 
8          Boursnell C           Boursnell             C           0000-0002-3494-4451   
9          Breaker RR            Breaker               RR          0000-0002-2165-536X   
10         Brown C               Brown                 C                                 
11         Burge SW              Burge                 SW          0000-0002-2506-927X   Burge S
12         Cartinhour SW         Cartinhour            SW                                
13         Chen A                Chen                  A                                 
14         Collins JA            Collins               JA                                

Transaction completed successfully

Five runs in succession with no failures.

@rtrussell
Copy link
Contributor

Are you running a modified version of the program?

As I noted in my previous post "When it works here (using the IP address) it gives the expected output"; using the IP address rather than the URL was the only change I made.

After changing line 314 to:

DIM payload&(1024)

What was the purpose of that change? I've run the program hundreds of times on different platforms (including the Pico W) and the 256-byte array has always been sufficient. I've never seen an error -4 (payload too large) reported.

@Memotech-Bill
Copy link
Owner Author

The version of the program I had was originally:

DIM payload&(65536)

@rtrussell
Copy link
Contributor

The version of the program I had was originally:

DIM payload&(65536)

Where did that come from? mysqllib.bbc has always had DIM payload&(255) right back to when it was first written in 2022. In any case I would never write 65536 since the array is one element longer than the specified subscript, so if written by me it would be payload&(65535) or more likely payload&(&FFFF).

@Memotech-Bill
Copy link
Owner Author

So I mis-remembered when typing the comment, if is 65535 not 65536.

65535 is what is currently in BBCSDL/lib/mysqllib.bbc in your repository.

@rtrussell
Copy link
Contributor

rtrussell commented Oct 22, 2024

65535 is what is currently in BBCSDL/lib/mysqllib.bbc in your repository.

There is some confusion here. In mysqllib.bbc there are two separate lines in which a LOCAL payload&() array is declared: in FN_sqlColumns there is a DIM payload&(255) and in FN_sqlFetch there is a DIM payload&(65535). The second one needs to be big, because one has no idea how big a database element might be and it therefore shouldn't limit the size any more than necessary. The 65536-byte array still fits when mysqldem is run on the Pico W.

The earlier, smaller, array is used when reading the column headings and therefore doesn't need to be so big. That's the one I assumed you were talking about, because your modification to payload&(1024) could easily be too small for a database element in the general case.

I see no reason for changing mysqllib.bbc from the version which is supplied with BBC BASIC for SDL 2.0 and all the other BBC BASIC Console Mode editions. Indeed, none of the libraries supplied with the Pico edition is different from those supplied with the other Console Mode editions (apart from socklib.bbc of course) because my build process copies the master libraries on top of any in your repository.

Incidentally I also delete the *.bas files which I pull from your repository because they are Linux-style text files with LF line endings, which don't display sensibly with *TYPE.

@rtrussell
Copy link
Contributor

Have you accidentally left some debugging code in socklib (or elsewhere)? I'm seeing this unwanted output when I connect:

H% = DEC23EC1
ipaddr = DEC23EC1 ^ipaddr = 2001E974

@Memotech-Bill
Copy link
Owner Author

Apologies, yes.

Fixed now.

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

No branches or pull requests

2 participants