created UCI commands for tcp listener

[xlar54]

Demo over at:

https://github.com/xlar54/ultimateii-dos-lib/tree/master/target

the d64 or the individual u-echoserver.prg file

It also handles disconnects without issue.

[xlar54]

FYI, not sure why, but the u2plus target will not build and I should be up to date with your code. Compiling just the niosapps worked though.

[xlar54]

Also, the crew on the FB channel asked for the port to be configurable. Would you want this maintained in the internal configuration, or sent by my library code? Two things concern me here... 1) you could use more ports later for other things and code needs to exist to prevent the listener from using those ports, and 2) there was mention of possibly accepting multiple connections on the same port. Considering use cases for that. I was only considering a single incoming connection.

So didnt know if you wanted to manage the incoming port on the config side, or just allow the developer to pick a port in my library and return a status of fail if its an invalid port from a list or such.

[markusC64]

FYI, not sure why, but the u2plus target will not build and I should be up to date with your code. Compiling just the niosapps worked though.

Works for me - I've just build the complete 1541 U2+ firmware from git master.

[GideonZ]

Nice proof of concept. Before actually unrolling these changes, let's make some extra steps:

• Users request multiple connections on a single port. The network stack supports this, but it means that the listening task should remain running even after the connection has been established.

• There seems to be only one instance now of the listener. Also this listener doesn't have a parameter; the port number cannot be set. How about creating an instance when performing the "start listen socket" command, taking the port as a parameter? Or, manage multiple tasks within one userlistener object, whichever you find more convenient.

• The commands should return information (success / fail). Also, it is necessary to have some sort of handle to stop the listening, if multiple instances exist. This could be the listening socket number, for example. But you will HAVE to check against a list of started listeners, as you need to make sure that the user does not close a listener socket, which was not a listener socket in the first place.

• Can you provide a rationale that this code is thread-safe? (I am not saying it is not, but it is part of the design..)

• What happens in the following scenario:

  • User sets up a listening socket
  • User 'closes' the listening socket
  • User connects to the socket that was just closed

• As GitHub clearly shows, this code has a mix of tabs and spaces. Please use tab-stop of 4, and convert to spaces, for proper indentation.

[xlar54]

Responding to each of the bullet items:

• Multiple connections concern me a bit because the developer will need to expect this. A BBS, for example, would only be able to deal with a single connection without a major overhaul. And I can see this as the primary use case. Certainly the listener could accept a second connection and a message sent to the new connection that the system is busy as such. So let me know if you're sure you want to try to support that.

• Yes, the port number should be a parameter. Will fix that. Just wanted to check with you and see if we want to maintain a common list of port numbers so that the developer can not use any of those. I can report back in the status register a failure with a text message of which ports are not available.

• Most of the command will return OK from the status register since they just alter a state value, but I should probably add some kind of check if the attempt to listen fails, I was just updating the state to "not listening" if the listen attempt fails. Same with close as you mention. Will fix that.

• Hmm.. I cant be certain on the thread safety issue. Again, was only anticipating single connections. Unless i implemented a pattern incorrectly which is quite possible.

• Closing the listener will disable the listener, so further connections would not succeed. (with current implementation of single connection only)

• Argh, yes. This one gets me all the time in various projects.

Thanks Gideon. If you can respond back - particularly around the multiple connections, that will get me in the right direction. Again, due to the primary use case of BBSs, I dont really see it worth trying to manage multiples. But if you want to support it anyway just in case, Im happy to take it further.

Thanks

Scott

[xlar54]

FYI, not sure why, but the u2plus target will not build and I should be up to date with your code. Compiling just the niosapps worked though.

Works for me - I've just build the complete 1541 U2+ firmware from git master.

Ok thanks Markus - Ill figure out where its going weird on my side.

Edit: Yes it was. Apparently recent changes I didnt have in my master branch. Sorry for that.

[xlar54]

Oh also - I wasnt sure which all Makefiles needed to be updated with the new .cc file for the listener. So I only did the two per this commit. (not sure if some are test ones, etc). Can you help me understand the various targets and what they build for? (in the /target/software directories)

[GideonZ]

Hi Scott,

Replying to you bullet-wise:

• The multiple connections is like opening multiple files. I think an OS should allow you to open multiple files. Whether to accept multiple connections on the same socket or not may indeed be under discussion. However, we may implement both, as a parameter: maximum number of connections for a listener. This way the software can decide whether it handles multiple connections or not. If it opens multiple ports, then for sure, it can handle multiple sockets as well.

• Common list of port numbers: I don't think that's necessary. The network stack already maintains this list. You cannot bind twice on a single port number. So we need to return what the bind function tells us.

• Closing the listener will disable the listener, you said. But from what I can see from the code, is that you delete the listener task. But I am not sure if the semaphore is properly disarmed. You know, when you are inside of a task that is waiting for something, and that task is removed, and then that something happens. What happens then, does the callback crash? I'd like to have this proven by review or documentation of FreeRTOS. Let's say FreeRTOS handles this case correctly (removing also the tasks waiting for semaphore from the lists internally), how does the IP stack handle this? I would say that the listening socket is not cleaned up or closed. Someone may even be able to connect to it, still!

I hope my remarks make sense.

Regards,
Gideon

[xlar54]

Yep all makes sense. Ok will take care of those changes, Thank you!

[xlar54]

I added the port number addition as an incoming parameter to the command, and better status on the listen command. I had an issue when flashing where it would for some reason go to the "out of the box" screen (1st boot) when I flashed it, so I dunno if the code is going left, or if my build is messed up. Im going to switch back to master, and make sure I can build and flash it.

[xlar54]

Yeah, seems there is a problem in building the U64 source. When flashing the master branch, it will go to the "Welcome to the Ultimate 64!" screen, and pressing the button on the side does nothing, requiring a flash via the adapter. Does this need an updated sof file or something in the external directory?

[markusC64]

Yeah, seems there is a problem in building the U64 source. When flashing the master branch, it will go to the "Welcome to the Ultimate 64!" screen, and pressing the button on the side does nothing, requiring a flash via the adapter. Does this need an updated sof file or something in the external directory?

I'd guess so. Looking at commit d718dd9 you see

    while (C64 :: c64_reset_detect())
        ;

and some VHDL code to enable reading reset... There is reason to belive that the same adjustment in the VHDL code is neccessary for the Ultimate 64, so yes - a new .sof should be needed.

[xlar54]

Gideon, could you hook us up with an updated sof file for the U64? Thanks

[GideonZ]

Hi Scott... Thank you for your changes. Please allow me some time, as I am a bit busy. I saw that you went all the way through flashing anew with the old firmware, also the old FPGA. If you have a USB Blaster, you can also just start a debug session from Eclipse and step through your code. Wouldn't that be easier?

I added the latest FPGA to the repository. This should fix your problem. I wasn't aware of this issue! Thanks.

[xlar54]

Thank you - and with me, take all the time you ever need. Im a software developer by day and can only imagine how busy you might be. Cant thank you enough.

[xlar54]

Ill have to stop here for the moment on handling multiple connections. There is one bug that Im struggling with - even though I close the listener socket, its still bound for some time. This seems to be by design. So youre right - not only does answering the phone the next time become a problem, but killing the task also then has a problem. And adding multiple connections compounds the problem further. Ill continue to see what I can do.