-
Notifications
You must be signed in to change notification settings - Fork 5
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
Comments
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 |
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:
Apart from the change to |
That is correct in principle. Doing make in the different folders just selects different default values. However, as noted later in that thread:
Looking at the CMakeLists.txt file, Building in the |
Hmm, "later in the thread" is only useful if there was a reason for me to read it, and there probably wasn't!
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? |
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:
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 A few words of explanation:
As a check, after building the program, you can use the command:
to show the signon message without having to load the program into a Pico. The significant part is the |
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 Ideally (but less important, because one can always restart the Pico) calling
That's similar to what I do, except that I think I use 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. |
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.
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.
|
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.
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? |
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
Moving 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 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 |
The current RAM memory map for the console/pico_w build is:
|
Let me try to clarify things with a couple of simple questions:
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. |
Yes
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.
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. |
OK. But - just so I'm absolutely clear - is that only if built in the
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.
Indeed, but if the background processing is completely disabled except for when socklib is active, they can hopefully all be lumped together. |
I currently maintain four builds:
For me it is most convenient to have separate folders for these. I simply have to do The "No Network" builds are retained because they do have more BASIC memory available than the "With Network" builds. If you look at the |
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 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. |
Which is currently the build in the
I have identified in the memory map a fairly large static allocation associated with network buffers:
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,
|
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:
CHAIN. |
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. |
It could be useful to return the original value of HIMEM to the caller, for example so you can restore it later:
In case it isn't obvious, the reserved memory for the networking buffer is at (the new value of) HIMEM, not at |
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:
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 ( More testing is needed. Should you wish to test, then |
Excellent, I was hopeful that should be possible.
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:
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, |
What is wrong with the following?
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:
If memory is allocated by moving HIMEM there are two options:
For a LOCAL structure, what is stored on the BASIC stack?
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. |
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.
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)?
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.
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:
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. |
Your code I have tried doing this from within C code. Testing with lanchat which calls
I find that when In C routine The only way I can see of making this work is if the code for As a side issue, I think the C variable |
"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:
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.
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. |
I can only assume that it works because the stack space reserved by In that case, how do I free that space in |
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:
|
Thanks for that. My version is:
And test results:
So clearly a minimum that can be reclaimed. |
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 |
Only to provide the user with more space for their files.
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:
to pull your latest source files. Alternately use
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. |
That sounds like a good solution, but unfortunately it doesn't work:
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. |
OK, I have pulled your latest repository into mine, then committed and pushed the change. I think that you now need to do:
It may not redirect the include files. I will have to check. |
My error :( The command should be:
And yes, it does redirect the include files as well as the C source. |
I'm sorry to report that doing this now builds an invalid filesystem_pwc.uf2 (at least, my Pico doesn't accept it):
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 |
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 The fundamental problem is that (in general) it is not easy (in the I need to give this some more thought. |
A bit of a cludge: I have added (and pushed) an option to Using this, my |
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. |
That is because my version used to zero fill all the space between the executable and the filesystem. I have revised this recently. |
I guessed that, mine never has. 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:
Is that anything I should worry about? |
Don't know. When I try it, I get:
Do any of the other network programs, such as lanchat work for you? |
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).
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.:
I've no idea what has changed. |
Yes it should do DNS lookups. It relies upon DHCP from the WiFi to provide netmask, gateway and DNS configuration. |
It should find 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 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)? |
The current version of wifi_scan is broken. It does not call Once you get a hard fault you will need to do a QUIT to re-initialise variables before any networking will work.
I see that the C SDK also lists:
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. |
Ah. I was about to release the latest build but it sounds like I should hold fire.
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:
OK. This is low priority, but could be useful as a future enhancement for those who have mesh systems. |
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. |
I have fixed an issue with DNS lookup. I am now failing to connect to the server due to a 10 second timeout. |
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. |
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:
Are you running a modified version of the program? Edit: After changing line 314 to:
I also obtain:
Five runs in succession with no failures. |
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.
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. |
The version of the program I had was originally:
|
Where did that come from? mysqllib.bbc has always had |
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. |
There is some confusion here. In mysqllib.bbc there are two separate lines in which a LOCAL 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 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. |
Have you accidentally left some debugging code in socklib (or elsewhere)? I'm seeing this unwanted output when I connect:
|
Apologies, yes. Fixed now. |
As per topic on Raspberry Pi forums, Richard Russel's build of PicoBB is missing network support routines.
However my build has them:
My build sequence is simply:
We need to identify what is different about the way Richard builds the program.
The text was updated successfully, but these errors were encountered: