search

m-labs / misoc

The original high performance and small footprint system-on-chip based on Migen™
https://m-labs.hk
Other
306 stars 86 forks source link

flterm should send characters immediately, not after user presses return #27

Closed sbourdeauducq closed 8 years ago

enjoy-digital commented 8 years ago

fixed via https://github.com/m-labs/misoc/commit/db3ef55434d93d99f2e87c742f022ed0676fdc97

I don't know why console.getkey() was waiting for return on Linux machines...

Now tested on Linux/Windows. Feel free to eventually cleanup getkey function for Linux, this is a copy/past from stackoverflow.

sbourdeauducq commented 8 years ago

Thanks for the quick fix. But can you please commit only clean code to misoc? The termios calls can be done only once at startup/termination of the program (as the C flterm did). The character function can also be removed and replaced with e.g. self.serial.read().decode().

enjoy-digital commented 8 years ago

I only tried to help you figuring out what was wrong (different behaviour of console.getkey() between machines). Thanks for your comment, I'm aware this is dirty, I'll see if I can improve that when I'll have more time for this.

sbourdeauducq commented 8 years ago

After reviewing flterm a bit more, I found many other problems (see IRC log). I could fix them myself, but I would like you to do it as an exercise and learn about e.g. Python's bytes and str. When you propose or commit code like this without being careful (I am convinced that this is carelessness or laziness, as you seem very capable), the code quality of Migen/MiSoC definitely suffers as I cannot catch every single problem when reviewing.

You probably have much insight yourself into where the issues are. For example, I'm pretty sure you did not really understand Python's bytes when pyserial returned you some, and you did not want to learn about them, so instead you hacked together messy/buggy code, did some cursory testing and called it a day. Please make the effort to understand how your program works in detail and write proper code, as myself and other contributors to migen/misoc/artiq do.

enjoy-digital commented 8 years ago

I just did a first cleanup pass. There is still the getkey functions(termios) and threads to cleanup. If you still see problems around bytes/str, can you report it to me?

sbourdeauducq commented 8 years ago

Much better, thanks! I don't actually understand how the thread starting/stopping is supposed to work but (1) threads are generally awful in Python in ways that often cannot be fixed (2) we will replace this part of the code when switching pyserial to asyncio. So the current solution is okay-ish.

enjoy-digital commented 8 years ago

Thanks for the comments, I did another cleanup. OK for the threads. I'll look at termios later.