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

MEGA65: uartmon_update() only processes a single monitor command, ignoring other maybe pending ones #220

Closed
gurcei opened this issue Dec 22, 2020 · 20 comments · Fixed by #268

Comments

@gurcei
Copy link
Contributor

gurcei commented Dec 22, 2020

Describe the bug
This was an observation discussed in this thread in discord:

In the typical case where a user is manually typing in serial monitor commands via m65dbg, these commands work fine in xemu. However, if tools like m65 or mega65ftp are used, such tools send multiple serial commands in quick succession. So by the time the uartmon_update() function is called, several commands have piled up. However uartmon_update() will only handle the first command found and ignore the rest.

Used version of the project
Can't recall the exact version I used at the time, but I believe it still occurs on the vic branch, and I had already dabbled in trying to fix it there, so will try continue on with that attempt.

To Reproduce

  • Run xemu with the "-uartmon uart.sock" argument
  • Run m65dbg with the "-l unix#path/to/xemu/uart.sock" argument
  • Run the 'ftp' command in m65dbg and watch it stall and choke

Expected behavior
For tools like m65 and mega65ftp to work successfully with xemu

Computer/Device (please complete the following information):

  • Device/Platform: PC
  • OS and its version: Windows 10 64 bit
@lgblgblgb
Copy link
Owner

lgblgblgb commented Dec 22, 2020

Hmm, I am not sure by heart, but actually it's possible (IIRC) that this was designed this way since otherwise the code is simple too "crude" and a much finer stuff have should be written. Which is actually WIP (since a while ...) to totally replace the monitor thing. I'll check it, not sure now at first read, how serious this is, but indeed it was not keep in mind to communicate with non-human controlled client, and a typical human can't easily issue commands more frequently than update function executes at least :)

Though I guess your description is not precise in regards of OS (you mentioned windows) and "to reproduce" (you mentioned uart.sock, but named socket is not available for windows). It should not make a difference, but I am not 100% sure since the possible minor differences between tcp/ip and unix domain named socket handling and their behaviour. Can you try to test this on eg Linux (or Mac?) using named socket, to see if it's really the same behaviour? Thanks!

Also please give me some instruction how I can test with so if I find something I can play with it while trying to fixing the problem (if it's possible at all with the current limited design!).

@lgblgblgb lgblgblgb self-assigned this Dec 22, 2020
@lgblgblgb lgblgblgb changed the title Let uartmon_update() handle multiple commands within a single call MEGA65: uartmon_update() only process a single monitor command, ignoring other maybe pending ones Dec 22, 2020
@gurcei
Copy link
Contributor Author

gurcei commented Dec 22, 2020

Yep, fine with your train of thought above, some of which you also described in the linked Discord thread too, but good to have those thoughts here also.

Perhaps another train of thought in the Discord thread worth repeating here is the backstory on how this topic came up on my radar. You had hinted that the possibility of getting mega65ftp working with xemu would be a nice thing, allowing for a common means to write content to the sd-card image, regardless of whether using xemu or real hardware.

So I set about giving that a try and ran into the problem described in this thread. So for now, I'd say that's a good way to replicate the issue.

  • Run xemu
  • Run latest m65dbg (which has mega65ftp embedded inside it), connect it to xemu over the unix socket
  • In m65dbg, type the command 'ftp' to attempt to jump into ftp-mode.

This will freeze up and stall. From my memories of debugging on the xemu side, I would see some commands chained together in succession and xemu only parsing the first command and skipping the ones after.

@gurcei
Copy link
Contributor Author

gurcei commented Dec 22, 2020

Additionally, I've made some headway with my attempt last night, splitting up the received string into separate commands to be parsed individually. Happy to keep nibbling at it, perhaps push it to my xemu fork, but if you've got the appetite to look into it, happy to put my efforts on pause.

gurcei pushed a commit to gurcei/xemu that referenced this issue Dec 23, 2020
…d, and splitting out multiple commands within a received string
gurcei pushed a commit to gurcei/xemu that referenced this issue Dec 23, 2020
…cations to get the newer output of the 'r' command that was needed for the mega65ftp conversation to work.

It got described in further detail in this discord post:
- https://discord.com/channels/719326990221574164/719326990221574168/774013669800542279
@gurcei
Copy link
Contributor Author

gurcei commented Dec 23, 2020

Ok, I've pushed what I've done so far into a branch on my forked repo, you're welcome to have a browse of what I'm up to there. If you're happy for me to keep pushing in that direction, no probs. Or if you have other ideas, no probs.

@lgblgblgb
Copy link
Owner

Uh! I haven't expected you to do so, to be honest, since I've planned other kind of changes mixed with these (so there my lament above with my usual elongated style ...). But thank you anyway, and I'll have a detailed look very soon!

@lgblgblgb
Copy link
Owner

Hopefully you don't mind I commented on the two commits above.

@gurcei
Copy link
Contributor Author

gurcei commented Dec 23, 2020

Sorry the commits came as a surprise. I remember we discussed the topic on Discord about a month ago in some of the discord thread links I mentioned. Much of my code change attempt is from that November era where I think I tried to put it on your radar, but then I went cold on it for a while and only came back to revisit it a day or two ago. Even back in November, you did hint to me that you were underway with another implementation, so no worries, that aspect was on my radar too.

So see this branch on my repo more as a preservation of that attempt, for your inspection, to assess if any of it has some merit/value in the end. I'll also use my present efforts as a way of temporarily getting over any hurdles of this xemu to mega65ftp conversation, so in a sense, pre-process and investigate some of the hurdles early.

@gurcei
Copy link
Contributor Author

gurcei commented Dec 26, 2020

Oh, I overlooked one of the edits you made to the comments asked for confirmation on my OS environment. Yes, I was using Windows, however, I had compiled xemu under cygwin (needing a few tweaks here and there), hence why I was able to use named sockets. If I get the chance, I could try named pipes on either mac osx or linux.

@lgblgblgb
Copy link
Owner

lgblgblgb commented Dec 26, 2020

Hmm, why cygwin, it's intended with mingw, much better, not external dll etc (or this is true for cygwin too? at least at the old times, cygwin needed an extra DLL ...) needed. With installing msys2 (ie, a "mingw distribution" native for windows), xemu compiles on windows natively without zero extra effort. As documented by README. The other advantage of mingw for Windows (ie msys2 is an easy to use distribution of mingw for windows) that is totally unified solution to compile for windows using both of linux (cross compilation) and windows (native compilation). Personally I really don't like cygwin since it's a full GNU-like environment for windows, with its overhead etc, while mingw as name suggest is "MINimal Gnu" only, and xemu was coded by me to meet that requirement.

@gurcei
Copy link
Contributor Author

gurcei commented Dec 26, 2020

Ah, the choice of cygwin was a remnant of my historical efforts. Checking from my logs, my first attempt to build xemu under cygwin was on 21/01/2017. It was from a period of time when you had just introduced the named-socket mechanism and you expressed concerns on whether the technique would work under windows. Hence why I at the time looked into getting it to build under cygwin. From memory, I don't recall the xemu project having a mingw build option back in 2017, only linux/macosx, hence my exploratory efforts at the time.

As for why I continued to use it, I suppose it was just familiar to me now as an option (the devil I knew), and the mingw build path is less familiar. I do have some mingw/msys stuff installed on my windows laptop, though I recently struggled to use it to build something that Paul was working on, so feel like mingw will be the 'devil I need to get more familiar with'.

So I can assure you the choice didn't really come about due to thinking one was better or worse than the other, though at the time, cygwin did have the named-socket support and mingw doesn't, so it probably swayed my decision back in 2017.

As for whether I should persist with cygwin, I'm open to moving on to mingw, though know there will some time taken to make that transition. So till then, where I can, I try to get by with cygwin.

@lgblgblgb
Copy link
Owner

lgblgblgb commented Dec 26, 2020

Well surely it's OK if you build Xemu yourself using cygwin, since it's your choice. What I worry about more that you want to push changes into Xemu checked with cygwin which may not work with mingw. At the other hand I don't want cygwin support in Xemu at all, since one build system is enough, well we have already even kind-of-TWO (cross compilation for windows, mingw native for windows), I find that totally useless to support more, in fact by contrast it causes more harm what it helps, then it needs maintain both, check for both, etc all the time. So do not misunderstand me please, I just say, on Xemu contribution, cygwin is not an option for legit reasons, not personal one ...

@gurcei
Copy link
Contributor Author

gurcei commented Dec 26, 2020

Ok, on this point I hear you. For my part, I'll say that at this stage I didn't have an intention to push to your repository, more to keep my efforts and intent on your radar.

As I reflect more on the 'why' behind my attempts here, it's to get mega65ftp behaviour working with xemu, in the hope that this will help with debugging ftp-related issues.

Why debug with xemu rather than hardware? It's due to me finding the hardware's serial monitor debugger behaviour to be somewhat broken lately (doing things like ignoring breakpoints). While Paul is aware of the shortcomings, it's not something he'll be able to address any time soon, as the uart monitor stuff got re-written by somebody else in verilog and it will take him time to get up to speed with it. Even I've tried absorbing some of it, and I won't get a full grip of it any time soon either.

So for my personal efforts in my branch, I'd like to just bash away with what I've got and get cygwin/xemu working with mega65ftp and assess such ftp issues.

Whether those efforts on my branch may be of use to you or not, not sure, maybe. Like you say, they may have been tainted with cygwin-isms that you don't want. On the other hand, they may resolve matters that were affecting both mingw and cygwin.

Anyway, you mentioned you were underway with a re-write of the uart-monitor stuff, please don't let me efforts be seen as an impediment to that, please continue as you see fit there.

@lgblgblgb
Copy link
Owner

Indeed, in theory debugging can be more easy with an emulator, since much more freedom you have what you can do, and the needed changes are more easy as well. OK, in theory :) In practice, it can be a mixed experience :)

gurcei pushed a commit to gurcei/xemu that referenced this issue Dec 27, 2020
…s in one receive-call.

It also handles the edge case of getting one full command followed by a partial command well (upon the next receive-call, it can complete the partial command and then execute it)
gurcei pushed a commit to gurcei/xemu that referenced this issue Dec 28, 2020
gurcei pushed a commit to gurcei/xemu that referenced this issue Dec 28, 2020
gurcei pushed a commit to gurcei/xemu that referenced this issue Jan 2, 2021
gurcei pushed a commit to gurcei/xemu that referenced this issue Jan 2, 2021
…t, and end address is just the lower 16-bits
@lgblgblgb
Copy link
Owner

Hi @gurcei ! I'm wondering how much progress has been done, do you want to PR in the near future so I would better to delay the release of the next stable, or you plan to do more first, and let's delay these things for later? Thanks.

@lgblgblgb lgblgblgb moved this from TODO to In Progress in MEGA65 emulator project Jan 10, 2021
@lgblgblgb lgblgblgb added the WIP label Jan 10, 2021
@gurcei
Copy link
Contributor Author

gurcei commented Jan 11, 2021

Hmm, what I have still feels a bit experimental, and it has evolved a bit beyond the original card's intention and become more related to simply "get mega65ftp (via m65dbg) to talk to xemu".

Maybe there's merits in getting it into the next stable, if it'd be nice to get at least some of this capability into the hands of the xemu user early, though I suspect there won't be much demand for it as yet.

I think in the long term, I'd like to do more on it, m65dbg's copy of mega65_ftp is a little old (from July 2020), so I'd eventually like to update it to the latest version Paul has been working on, which seems to require a new bitstream with some newly added sd-card stuff that I'm not familiar with yet (my old m65dbg+ftp fails to work with it).

So it'd be nice to eventually have the newer ftp implementation working for both xemu and hardware, but think it might be a while away before I can sink my teeth too deep into this.

Given that, maybe it's best to not let this card hold you back from your next stable release.

@lgblgblgb
Copy link
Owner

Ok, just asking, no rushing :) And thanks for the "status report" or whatever we want to call that ;)

gurcei pushed a commit to gurcei/xemu that referenced this issue Oct 22, 2023
(so if you specify -uartmon :4510 it will open port 4510 and 4511)
gurcei pushed a commit to gurcei/xemu that referenced this issue Oct 22, 2023
… for mega65_ftp actions like 'get MYDISK.D81'.
gurcei pushed a commit to gurcei/xemu that referenced this issue Oct 22, 2023
gurcei pushed a commit to gurcei/xemu that referenced this issue Oct 22, 2023
gurcei pushed a commit to gurcei/xemu that referenced this issue Oct 22, 2023
gurcei pushed a commit to gurcei/xemu that referenced this issue Oct 22, 2023
gurcei pushed a commit to gurcei/xemu that referenced this issue Oct 22, 2023
gurcei pushed a commit to gurcei/xemu that referenced this issue Oct 22, 2023
gurcei pushed a commit to gurcei/xemu that referenced this issue Oct 22, 2023
lgblgblgb referenced this issue Dec 28, 2023
…o the hardware (particularly the extra map-mask bits in MAPL, MAPH)
gurcei pushed a commit to gurcei/xemu that referenced this issue May 15, 2024
…hod, and splitting out multiple commands within a received string
gurcei pushed a commit to gurcei/xemu that referenced this issue May 15, 2024
…cations to get the newer output of the 'r' command that was needed for the mega65ftp conversation to work.

It got described in further detail in this discord post:
- https://discord.com/channels/719326990221574164/719326990221574168/774013669800542279
gurcei pushed a commit to gurcei/xemu that referenced this issue May 15, 2024
…s in one receive-call.

It also handles the edge case of getting one full command followed by a partial command well (upon the next receive-call, it can complete the partial command and then execute it)
gurcei pushed a commit to gurcei/xemu that referenced this issue May 15, 2024
gurcei pushed a commit to gurcei/xemu that referenced this issue May 15, 2024
gurcei pushed a commit to gurcei/xemu that referenced this issue May 15, 2024
gurcei pushed a commit to gurcei/xemu that referenced this issue May 15, 2024
…bit, and end address is just the lower 16-bits
gurcei pushed a commit to gurcei/xemu that referenced this issue May 15, 2024
…mory (cpu context). My prior effort made all the 777 into zeros, so I've fixed that now.
gurcei pushed a commit to gurcei/xemu that referenced this issue May 15, 2024
(so if you specify -uartmon :4510 it will open port 4510 and 4511)
gurcei pushed a commit to gurcei/xemu that referenced this issue May 15, 2024
…re closer to the hardware (particularly the extra map-mask bits in MAPL, MAPH)
gurcei pushed a commit to gurcei/xemu that referenced this issue May 15, 2024
… for mega65_ftp actions like 'get MYDISK.D81'.
gurcei pushed a commit to gurcei/xemu that referenced this issue May 15, 2024
gurcei pushed a commit to gurcei/xemu that referenced this issue May 15, 2024
gurcei pushed a commit to gurcei/xemu that referenced this issue May 15, 2024
gurcei pushed a commit to gurcei/xemu that referenced this issue May 15, 2024
gurcei pushed a commit to gurcei/xemu that referenced this issue May 15, 2024
gurcei pushed a commit to gurcei/xemu that referenced this issue May 15, 2024
gurcei pushed a commit to gurcei/xemu that referenced this issue May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment