Portable readline with callback required #1028

stefanrueger · 2022-07-15T18:02:13Z

Terminal mode already gives a radically different user experience depending on which OS it is compiled for and whether or not the readline library has been compiled with AVRDUDE. I plan to enable the terminal to work with bootloaders that need being kept alive lest the WDT resets them. One working solution is using the readline library with character callbacks. This works like a charm in Linux.

Here is a standalone small C termreadline.c.txt
file that demonstrates how this works. Just compile $ cc termreadline.c -lreadline and give it a go:

echo quit | a.out 2>&1 | cat
avrdude> quit
quit: bye

If you use it interactively, every 1.8 s a dot should appear, which indicates where the code could keep the bootloader alive.

So, here the challenge for the coders amongst us: can we get this piece of code working for Windows, MacOS and all other platforms and compilers we hope that AVRDUDE will run on?

The text was updated successfully, but these errors were encountered:

dl8dtl · 2022-07-15T19:17:22Z

MacOS is a UNIX (even a certified one, IIRC).
As such, the usual Posix descriptor methods do work there without problems.
The challenge will be Windows here, because they do this in a much different way.

Other remarks (I know it's not production code, but nevertheless):

use poll() instead of select() (Posix interface)
int readytoread() declares a function taking an arbitrary number and kind of arguments; use int readytoread(void) (this is different in C++)
prefer bool over int for boolean operations (and true and false then)

stefanrueger · 2022-07-15T19:50:07Z

Cool as! So, can we expect a Posix-compliant version from you @dl8dtl that is likely to work on UNIX-type systems? That would reduce the problem space already enormously.

dl8dtl · 2022-07-15T19:54:58Z

If you replace select() by poll(), I think your version would already be Posix.
(Most Posix systems will also accept the select() interface, but when standardizing, it has been superseded by poll().)

For WinAPI, we need someone knowledgable about it, to propose something that can then be #ifdefed for both. The poll() thingie is only a minor issue, just a bit of typing work.

dl8dtl · 2022-07-15T20:24:29Z

Here's the diff for poll vs. select, it's really pretty trivial.

--- termreadline.c.orig 2022-07-15 21:10:55.655980000 +0200
+++ termreadline.c      2022-07-15 22:22:47.569289000 +0200
@@ -3,6 +3,7 @@
 #include <readline/readline.h>
 #include <readline/history.h>
 #include <unistd.h>
+#include <poll.h>
 
 // Standalone readline test
 
@@ -51,12 +52,15 @@
 
 // Any character in standard input available?
 static int readytoread() {
-  struct timeval tv = { 0L, 0L };
-  fd_set fds;
-  FD_ZERO(&fds);
-  FD_SET(0, &fds);
+  struct pollfd fds[1] = {
+    {
+      .fd = 0,
+      .events = POLLIN | POLLPRI | POLLERR | POLLHUP,
+      .revents = 0
+    }
+  };
 
-  return select(1, &fds, NULL, NULL, &tv) > 0;
+  return poll(fds, 1, 0) > 0;
 }
 
 // Callback processes commands whenever readline() has finished

(Edit: I forgot the > 0 comparison before.)

dl8dtl · 2022-07-15T20:27:51Z

Maybe revents should be checked for one of the exceptional events as well.

mcuee · 2022-07-16T01:53:43Z

@stefanrueger
Windows can be a challege.

For example, I can build under MSYS2 mingw64 of your code with a quick patch but the result exe does not work properly.

$ diff termreadline.c termreadline_sr.c
6a7,10
> #if defined(WIN32)
> #include <winsock2.h>
> #endif
>

$ echo quit | ./termreadline_sr.exe 2>&1 | cat
avrdude> ................................... (just keep typing `.` and not accepting CTRL-C)

MINGW64 /c/work/avr/avrdude_test/readline
$ ./termreadline_sr.exe
avrdude> ... (hit CTRL-C)
MINGW64 /c/work/avr/avrdude_test/readline
$ quitquit

mcuee · 2022-07-16T02:10:40Z

If you replace select() by poll(), I think your version would already be Posix. (Most Posix systems will also accept the select() interface, but when standardizing, it has been superseded by poll().)

For WinAPI, we need someone knowledgable about it, to propose something that can then be #ifdefed for both. The poll() thingie is only a minor issue, just a bit of typing work.

@dl8dtl and @stefanrueger
FYI libusb older version may have the codes you can use.
Ref: https://github.com/libusb/libusb/tree/v1.0.23/libusb/os
https://github.com/libusb/libusb/blob/v1.0.23/libusb/os/poll_windows.c
https://github.com/libusb/libusb/blob/v1.0.23/libusb/os/poll_posix.c

Not so sure if simpler one works or not.
https://raw.githubusercontent.com/libressl-portable/portable/master/include/compat/poll.h

mcuee · 2022-07-16T02:48:43Z

Not so sure if simpler one works or not.
https://raw.githubusercontent.com/libressl-portable/portable/master/include/compat/poll.h

Maybe it can be used with minor modifications.

$ diff termreadline_sr.c termreadline.c
7,12d6
< #if defined(WIN32)
< #include "poll.h"
< #else
< #include <poll.h>
< #endif
<

$ gcc -o termreadline_sr termreadline_sr.c -lreadline -lws2_32

$ gcc -o termreadline_jw termreadline_jw.c -lreadline -lws2_32
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/11.3.0/../../../../x86_64-w64-mingw32/bin/ld.exe: 
C:\msys64\tmp\ccKZspNO.o:termreadline_j:(.text+0x1c0): undefined reference to `poll'
collect2.exe: error: ld returned 1 exit status

termreadline_jw.c.txt
termreadline_sr.c.txt

For Linux, both versions work fine.

mcuee@UbuntuSwift3:~/build/avr/readline$ echo quit | ./termreadline_sr 2>&1 | cat
avrdude> quit
quit: bye

mcuee@UbuntuSwift3:~/build/avr/readline$ echo quit | ./termreadline_jw 2>&1 | cat
avrdude> quit
quit: bye

stefanrueger · 2022-07-16T09:40:10Z

So, we have a solution that works in UNIXes. And we have an Any character ready? function for Windows. Progress! Here my summary of @mcuee and @dl8dtl's ideas:

#include "poll.h"

// Any character in standard input available?
static int readytoread(void) {
  struct pollfd fds = {   
    .fd = 0,
    .events = POLLIN | POLLPRI,
    .revents = 0,
  };

  return poll(&fds, 1, 0) > 0;
}

@mcuee, It'll be sufficient to #include "poll.h" because "abc.h" falls back on <abc.h> if it wasn't found in local places.

@dl8dtl re the evaluating revents, I don't know what kind of things we should look out for to harden the code against; I'd run with above and see whether there are cases that need checking in practice. I've removed POLLHUP and POLLERR from events, as they are ignored there, but checked for and reported in revents anyway (according to man poll).

stefanrueger · 2022-07-16T09:45:12Z

Is there a way in cmake or otherwise to require/install the readline library? The GNU readline library seems widely available in UNIX type systems and on MacOS. Hopefully, someone can point out a compatible readline library in Windows, too.

Reason for this that otherwise we would need to provide a poor man's readline() with callback. Can be done, eg, using a reasonable source of fgets() and transform it to a function that gets characters fed in a similar way to readline's callback. Only, this is bound to be a poor terminal input experience. The readline() source has 52k lines (8k of which in configure) and caters well for all sorts of systems and I/O (in comparison AVRDUDE source has 72k lines 20k of which in avrdude.conf). Any 20-liner replacement will suck out of necessity.

[edit: AVRDUDE 7.0 has 78k lines]

mcuee · 2022-07-16T09:49:59Z

@stefanrueger
As mentioned before that mingw32/64 have readline.
https://github.com/msys2/MINGW-packages/tree/master/mingw-w64-readline

But MinGW does not have native good "poll.h" so you need to see if the free alternative implementation works or not. Usually the simple ones are based on winsock2.
https://docs.microsoft.com/en-us/windows/win32/api/winsock2/

For MSVC, there is a vcpkg formula which can be integrated into CMake but it may be easier to use @mariusgreuel's method of having a avrdude specific readline repo for MSVC build.
https://github.com/microsoft/vcpkg/tree/master/ports/readline
https://github.com/microsoft/vcpkg/tree/master/ports/readline-win32
https://github.com/lltcggie/readline (the real codes are here).

mcuee · 2022-07-16T10:21:33Z

@stefanrueger
Here is the output of MSY2mingw64 build of PR 1025 using readline. It does work similar to Linux with readline support.

 MINGW64 /c/work/avr/avrdude_test/avrdude_sr
$ ./build_mingw64_nt-10.0-22000/src/avrdude.exe -c usbasp -qqp m328p -t
avrdude> erase
avrdude.exe: erasing chip
avrdude> write eeprom 0 0xdeadbeef 'a' 'v' 'r' " Hello World\n" 0x00000002
avrdude> read eeprom 0 0x20
0000  ef be ad de 61 76 72 20  48 65 6c 6c 6f 20 57 6f  |....avr Hello Wo|
0010  72 6c 64 0a 00 02 00 00  00 ff ff ff ff ff ff ff  |rld ............|

avrdude> quit

$ git diff
diff --git a/src/term.c b/src/term.c
index f02f95c..eb98c14 100644
--- a/src/term.c
+++ b/src/term.c
@@ -1476,7 +1476,7 @@ int terminal_mode(PROGRAMMER * pgm, struct avrpart * p)
       return argc;
     }

-#if !defined(HAVE_LIBREADLINE) || defined(WIN32) || defined(__APPLE__)
+#if !defined(HAVE_LIBREADLINE)
     fprintf(stdout, ">>> ");
     for (int i=0; i<argc; i++)
       fprintf(stdout, "%s ", argv[i]);

mcuee · 2022-07-16T13:50:43Z

Under macOS homebrew, they seem to prefer you to use pkg-config or export the compiler flags.

mcuee@mcuees-Mac-mini readline % brew info readline
readline: stable 8.1.2 (bottled) [keg-only]
Library for command-line editing
https://tiswww.case.edu/php/chet/readline/rltop.html
/opt/homebrew/Cellar/readline/8.1.2 (48 files, 1.7MB)
  Poured from bottle on 2022-03-08 at 16:13:22
From: https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/readline.rb
License: GPL-3.0-or-later
==> Caveats
readline is keg-only, which means it was not symlinked into /opt/homebrew,
because macOS provides BSD libedit.

For compilers to find readline you may need to set:
  export LDFLAGS="-L/opt/homebrew/opt/readline/lib"
  export CPPFLAGS="-I/opt/homebrew/opt/readline/include"

For pkg-config to find readline you may need to set:
  export PKG_CONFIG_PATH="/opt/homebrew/opt/readline/lib/pkgconfig"

==> Analytics
install: 418,591 (30 days), 1,327,493 (90 days), 7,175,307 (365 days)
install-on-request: 14,060 (30 days), 44,992 (90 days), 231,983 (365 days)
build-error: 7 (30 days)

mcuee@mcuees-Mac-mini readline % export PKG_CONFIG_PATH="/opt/homebrew/opt/readline/lib/pkgconfig"
mcuee@mcuees-Mac-mini readline % gcc `pkg-config --cflags --libs readline` termreadline_sr.c -o termreadline_sr
termreadline_sr.c:40:1: warning: non-void function does not return a value [-Wreturn-type]
}
^
1 warning generated.
mcuee@mcuees-Mac-mini readline % gcc `pkg-config --cflags --libs readline` termreadline_jw.c -o termreadline_jw
termreadline_jw.c:40:1: warning: non-void function does not return a value [-Wreturn-type]
}
^
1 warning generated.

mcuee@mcuees-Mac-mini readline % echo quit | ./termreadline_sr 2>&1 | cat                                      
avrdude> quit
quit: bye

mcuee@mcuees-Mac-mini readline % echo quit | ./termreadline_jw 2>&1 | cat                                      
avrdude> quit
quit: bye

mcuee · 2022-07-17T02:54:54Z

@stefanrueger
BTW, your partdesc branch seems to be useful as well but it is not portable under Windows since fnmatch is not avaiable for MinGW or MSVC. It works well under Linux and macOS.

dl8dtl · 2022-07-17T15:01:36Z

Since it is only used to match a simple pattern on the part name, it's probably easy enough to write a replacement.

mcuee · 2022-07-17T23:56:15Z

Since it is only used to match a simple pattern on the part name, it's probably easy enough to write a replacement.

Indeed.
https://github.com/stefanrueger/avrdude/blob/partdesc/src/developer_opts.c#L808

    // pattern match the name of the part with command line: FMP_CASEFOLD not available here :(
    if(fnmatch(partdesc, p->desc, 0) && fnmatch(partdesc, p->id, 0))
      continue;

Maybe the following can work.
https://stackoverflow.com/questions/35877738/windows-fnmatch-substitute
https://docs.microsoft.com/en-us/windows/win32/api/shlwapi/nf-shlwapi-pathmatchspecw

#ifdef _WIN32
            wchar_t wname[1024];
            wchar_t wmask[1024];

            size_t outsize;
            mbstowcs_s(&outsize, wname, de->d_name, strlen(de->d_name) + 1);
            mbstowcs_s(&outsize, wmask, wildcard.c_str(), strlen(wildcard.c_str()) + 1);

            if (PathMatchSpecW(wname, wmask))
                result.push_back (std::string(de->d_name));
            else
                continue;
#else
                if (fnmatch(wildcard.c_str(), de->d_name, 0))
                    continue;
                else
                    result.push_back (std::string(de->d_name));
#endif

stefanrueger · 2022-07-18T10:06:48Z

fnmatch

@mcuee Thanks for looking at the partdesc branch and catching portability issues before I could even propose these developer options (amongst others, describe parts in a way that can be used as input for avrdude.conf). The tip with PathMatchSpecW() looks great.

mcuee · 2022-07-31T02:50:26Z

PR #1025 has been merged. PR #1040 has been proposed for the partdesc branch and now it is good for portability.

Going back to the topic of readline, once the issues are sorted out (eg: proper detection of readline using CMake; or mandatory requirement of readline -- need to sort out MSVC), then the following quick fix can be changed to properly support macOS and MinGW when readline support is available.

avrdude/src/term.c

Lines 1479 to 1484 in affe4cb

    
           #if !defined(HAVE_LIBREADLINE) || defined(WIN32) || defined(__APPLE__) 
        
               fprintf(stdout, ">>> "); 
        
               for (int i=0; i<argc; i++) 
        
                 fprintf(stdout, "%s ", argv[i]); 
        
               fprintf(stdout, "\n"); 
        
           #endif

mcuee · 2022-09-05T12:59:30Z

@mariusgreuel
Maybe you can help answer the question related to using readline with CMake. Thanks.

mariusgreuel · 2022-09-05T20:32:02Z

@stefanrueger While looking at you demo, I am wondering what your requirements are. If you just want a function called periodically, this is not going to be a problem on Windows. However, you probably need another thread, and this is not how you implemented it using libreadline, which I take is single-threaded by design?

I am not aware of a readline package for Windows. Googling gives me a GNU libreadline package for Windows from 2008. Lots of code, probably unmaintained, not sure if it will work at all.

I did a little research, and it looks like if you do a fgets(), you will end of with a call to ReadFile(), which blocks until \n is received. In other words, you cannot sneak in between, time-out, or handle another event. If you want a "peek, process, sleep" cycle like you showed in a demo, you need to implement the whole command-line editing stuff yourself, just as libreadline does. One can certainly do this using the Windows console API (and perhaps this is what libreadline for Windows does), but that is going to be a mammoth task, given what the Windows console is capable of.

So, would it help to have a simple timer calling you on a separate thread?

mcuee · 2022-09-06T00:01:41Z

I am not aware of a readline package for Windows. Googling gives me a GNU libreadline package for Windows from 2008. Lots of code, probably unmaintained, not sure if it will work at all.

Not exactly very new (actually very old) but at least part of vcpkg.
https://github.com/microsoft/vcpkg/tree/master/ports/readline
https://github.com/microsoft/vcpkg/tree/master/ports/readline-win32
https://github.com/lltcggie/readline (the real codes are here).

stefanrueger · 2022-09-06T08:40:18Z

@mariusgreuel

Would it help to have simple timer calling you on a separate thread?

no

TLDR; requirements for Windows:

readytoread(void) from termreadlinev2.c works, implemented with POSIX poll()
GNU readline library with rl_set_prompt(), add_history(), rl_callback_handler_instal(), rl_callback_read_char() and rl_callback_handler_remove()

This to give Windows users the same experience as UN*x users with respect to (1) running AVRDUDE terminal with bootloaders and (2) input for terminal mode.

The long of it. I have working code for LINUX (not yet in mainline) that relies on the mechanisms in the demo termreadlinev2.c.

The main objective is to enable the terminal with bootloaders, which typically time out after 250..8000 ms of inactivity. One way (actually, the only one I could come up with) is to send every 100 ms a GETSYNC command to the bootloader (a kind of NOP) while waiting for terminal input from the user. At other times some processing and communications with the bootloader takes place anyway, which resets the WDT of the bootloader. Here is the snag: one cannot just send a GETSYNC command every 100 ms in another thread: that is bound to be in the middle of some useful comms with the bootloader at some point, and the interspersed GETSYNC command would break that comms. So it is natural to process GETSYNC handshakes during the wait for characters (and also only start processing the fully read input line once any just sent GETSYNC command is finished).
The secondary objective is to get every user of AVRDUDE the same terminal experience irrespective of the operating system, for which a readline library is defo cool (history, line editing, correct echo on command line, ...). Turns out, gnu readline is an excellent library, which would already have been excellent in 2008 (at which time it would have been some 20 years old). I have no qualms using a 2008 version, and I predict that all the functions listed above will be there. GNU readline is large and feature rich, but also old, venerable and mature (arguably one the most-used library routine that people didn't know they used).

Failing 2. I can write a poor man's substitute for fgets(), which serves the first objective assuming int readytoread(void) works under Windows. I would write this in terms of single character reads using the kernel read() that won't block because read() would only be called when readytoread() asserts one char can be read without blocking. But that's certainly not the same experience as readline() for which my demo implements non-blocking reads.

stefanrueger · 2022-09-06T08:52:19Z

Would it help to have simple timer calling you on a separate thread?

Yes, if there is another readline() library that is incompatible with GNU readline. In which case a separate timer in another thread could send GETSYNC commands to the bootloader whilst readline() is active; requires synchronisation points between start/end of readline() and the other thread.

stefanrueger · 2022-09-06T08:52:58Z

My preference is GNU readline (because even an old version will be great)

mcuee · 2022-09-06T09:44:17Z

My preference is GNU readline (because even an old version will be great)

The vcpkg readline-win32 package is a port of GNU readline 5.0 for Windows. readline 5.0 was rather old since it was released in 2004.

Just wondering if you can take a look at the alternative which does support Windows along with Linux and macOS.
https://github.com/AmokHuginnsson/replxx

mariusgreuel · 2022-09-06T15:07:58Z

https://github.com/lltcggie/readline

@mcuee Thanks for the links! I just tried the readline-win32 package and it worked (somewhat) out of the box. I can compile with a few changes, type characters and the callback works! However, when I pressed Ctrl-R is crashed. Seems like it is C code from stone-age, not ready for Windows 64-bit pointers :-(.

one cannot just send a GETSYNC command every 100 ms in another thread

@stefanrueger Absolutely! If multi-threaded, we would have to do the synchronization ourselves, which might be a bit of a problem, since I just got rid of pthreads during the MSVC port and I am not keen on bringing it back.

The secondary objective is to get every user of AVRDUDE the same terminal experience irrespective of the operating system

Not sure if I would agree here. IMHO, people are expecting it to work like it usually does on their OS. For command-line applications, Windows offers a built-in readline equivalent. Besides basic editing, there is the wildly popular F7 shortcut, which brings up the history. You get that and other stuff for free when you do a fgets() (*). With libreadline I get nothing when I press F7, but of course Ctrl-R, etc.

That said, I think the people who use avrdude in terminal mode can manage using libreadline. I guess I just prefer what the system offers, just saying.

(*) Turns out F7 is broken in avrdude using fgets(). Need to look into this...

mariusgreuel · 2022-09-06T16:01:22Z

Turns out F7 is broken in avrdude

See #1097

mariusgreuel · 2022-09-06T16:11:50Z

termreadline demo using Windows and vcpkg readline-win32 for Visual Studio 2022.
https://github.com/mariusgreuel/termreadline

vcpkg install readline-win32
vcpkg install readline-win32:x64-windows

Adjust CMAKE_TOOLCHAIN_FILE in CMakeSettings.json to point to your vcpkg.

stefanrueger · 2022-09-06T18:23:53Z

@mariusgreuel
Thanks for your port of the termreadline demo for Windows. Really cool.

same terminal experience irrespective of the operating system

people are expecting it to work like it usually does on their OS

We are talking cross purposes. Same experience for me means having both, terminal for bootloaders and command line editing. As fgets() contains command line editing in Widows but no callback for asynchronous GETSYNC calls to the bootloader, it looks like either terminal for bootloaders or command line editing in Windows.

My preference is still to go along with the GNU readline library, as it seems readily available in Linux, FreeBSD, MacOS and --- with your port --- also Windows. Has the benefit of (almost) the same code everywhere. Happy to follow your lead for alternative solutions in Windows, though. It will be a few weeks before I present a PR implementing terminal for bootloaders, so perhaps best to decide then how this looks in Windows.

For the time being this has given me confidence that we can rely on availability of readline() in all OS that we cater for.

mariusgreuel · 2022-09-06T20:19:37Z

My preference is still to go along with the GNU readline library

I have to admit, your solution is dirt-simple.

this has given me confidence that we can rely on availability of readline() in all OS that we cater for

I agree. No matter how the Windows solution is going to look like, I will not require a redesign of your code.

stefanrueger · 2022-09-15T08:27:24Z

Quick question @mariusgreuel In your code

https://github.com/mariusgreuel/termreadline/blob/792caef19efa003487b18e8b2684090299a88b25/termreadline.c#L62-L65

does _kbhit() only look at the physical keyboard or does it look at standard input (file descriptor 0)? This routine really needs to figure out whether the next read(0, ...) will block or not be it from a pipe or redirected from a file.

stefanrueger · 2022-09-16T16:04:12Z

Reading up on _kbhit() makes me think it does not work with stdin on Windows (and rather returns the whether reading from that is defined as console blocks). So, I suspect @mariusgreuel 's port needs to be changed so that

echo quit | termreadline
avrdude> quit
quit: bye

works in Windows.

mariusgreuel · 2022-09-17T12:55:27Z

This routine really needs to figure out whether the next read(0, ...) will block or not be it from a pipe or redirected from a file.

I see what you mean. _kbhit() is not going to work, just tried it.

mariusgreuel · 2022-09-17T13:49:20Z

Hm, this was a little more complicated than expected. Checking for an event via WaitForSingleObject(hStdin, 0) (the equivalent of select()) did not work, because libreadline does not handle all console events and then blocks, for instance when a key-up event is in the queue.

I managed to make it work by inspecting the libreadline code, and filtering all events but those that are actually handled. I pushed another change to my repo, however, it only partially works: The EOF detection does not return NULL, so an explicit quit is required in the input. Saving this for some other time.

https://github.com/mariusgreuel/termreadline

stefanrueger · 2022-09-20T21:39:14Z

I pushed another change to my repo

Thanks, but I am not seeing a change in your termreadline port?

mariusgreuel · 2022-09-21T11:14:11Z

Thanks, but I am not seeing a change in your termreadline port?

Ah, forgot to push it ;-)

stefanrueger · 2022-09-21T17:00:07Z

Wow, great prototyping and good progress.

I am sure you are aware that now that readytoread() returns -1 on error, and no longer just 0 and 1, this requires a change of how it is called in https://github.com/mariusgreuel/termreadline/blob/fc05ee8082d90aaa42f541df1d823499bc45f13c/termreadline.c#L153-L154

I predict (but don't know) that this works when the input comes from a file, and even the EOF detection will work there. And, yes, suspect, this needs some further work if input is from a pipeline: https://github.com/mariusgreuel/termreadline/blob/fc05ee8082d90aaa42f541df1d823499bc45f13c/termreadline.c#L74-L78

Reading from a pipeline can make the process sleep. One needs a peek ahead there...

stefanrueger · 2022-09-23T23:38:22Z

The EOF detection does not return NULL

Is that in the terminal, when reading from file or when reading from a pipeline? In the terminal it's usually a certain character at the beginning of a line that signifies EOF. Is that still ctrl-Z in Windows?

mariusgreuel · 2022-10-08T16:12:15Z

and even the EOF detection will work there

I think the problem is the WIN32 implementation of rl_getc(): On EOF, it does not return EOF but zero.
https://github.com/mariusgreuel/readline/blob/vs/src/readline/5.0/readline-5.0-src/input.c#L618-L620

It seems that the library needs some work, but it appears that it can be done with a reasonable amount of effort. I will fix the library when I find some time.

mcuee · 2022-10-21T01:50:40Z

@stefanrueger

Just wondering if it is possible to support BSD libedit (https://thrysoee.dk/editline/) under macOS without using readline.

Revamp terminal output (progress bar, callback and stdout/stderr) #1132 (comment)

Of course the easier solution is to use readline with some CMake magic that hopefully @mariusgreuel can help out.

stefanrueger · 2022-10-21T23:01:08Z

@mcuee Have you tried compiling PR #1132 with libedit instead of libreadline? Really, the important bit is the ability to do callbacks:

rl_callback_handler_install("avrdude> ", term_gotline);
rl_callback_read_char();
rl_callback_handler_remove();

If these three functions exist, and add_history(cmdstr), then all should be good.

mcuee · 2022-10-22T01:37:31Z

@mcuee Have you tried compiling PR #1132 with libedit instead of libreadline?

Ah, maybe I did not make it clear enough. Under macOS, PR #1132 will use libedit. I have not figured how to force it to use readline.

Revamp terminal output (progress bar, callback and stdout/stderr) #1132 (comment)

mcuee@mcuees-Mac-mini avrdude_bin % otool -L avrdude_pr1132_libedit 
avrdude_pr1132_libedit:
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1311.100.3)
	/opt/homebrew/opt/libusb-compat/lib/libusb-0.1.4.dylib (compatibility version 9.0.0, current version 9.4.0)
	/opt/homebrew/opt/libusb/lib/libusb-1.0.0.dylib (compatibility version 4.0.0, current version 4.0.0)
	/opt/homebrew/opt/libhid/lib/libhid.0.dylib (compatibility version 1.0.0, current version 1.0.0)
	/opt/homebrew/opt/hidapi/lib/libhidapi.0.dylib (compatibility version 0.0.0, current version 0.12.0)
	/opt/homebrew/opt/libftdi0/lib/libftdi.1.dylib (compatibility version 22.0.0, current version 22.0.0)
	/opt/homebrew/opt/libftdi/lib/libftdi1.2.dylib (compatibility version 2.0.0, current version 2.5.0)
	/usr/lib/libedit.3.dylib (compatibility version 2.0.0, current version 3.0.0)

The result avrdude is not working very well -- I need to hit RETURN after every commands. So there are some subtle differences between libedit and readline.

Revamp terminal output (progress bar, callback and stdout/stderr) #1132 (comment)

Really, the important bit is the ability to do callbacks:

rl_callback_handler_install("avrdude> ", term_gotline);

rl_callback_read_char();

rl_callback_handler_remove();

If these three functions exist, and add_history(cmdstr), then all should be good.

Yes all these functions exist in libedit. Take note libedit does not support Windows. It does support Linux, macOS and BSDs.
Reference:
https://thrysoee.dk/editline/
http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libedit/readline/readline.h?rev=1.53&content-type=text/x-cvsweb-markup&sortby=date

mcuee · 2022-10-22T03:17:52Z

I have not figured how to force it to use readline.

I have figured out a way to force it to use readline, but it is a manual process (similar to what I do under Windows using MSYS2). Once using readline and not libedit, PR #1132 works well under macOS.

Revamp terminal output (progress bar, callback and stdout/stderr) #1132 (comment)

mcuee · 2022-10-22T13:26:08Z

@stefanrueger
If you want to giev libedit a try, you can easily do that under Linux by install libedit development package (`sudo apt install libedit-dev" under Ubuntu/Debian).

 2043  cmake -D CMAKE_BUILD_TYPE=RelWithDebInfo 
-D HAVE_LIBREADLINE:FILEPATH=/usr/lib/x86_64-linux-gnu/libedit.so  -B build_linux_libedit
 2044  cmake --build build_linux_libedit
 2045  ldd ./build_linux_libedit/src/avrdude

mcuee@UbuntuSwift3:~/build/avr/avrdude_sr$ ldd ./build_linux_libedit/src/avrdude
	linux-vdso.so.1 (0x00007ffcb1fd9000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fbc8c12f000)
	libelf.so.1 => /lib/x86_64-linux-gnu/libelf.so.1 (0x00007fbc8c113000)
	libusb-0.1.so.4 => /usr/local/lib/libusb-0.1.so.4 (0x00007fbc8c10b000)
	libusb-1.0.so.0 => /usr/local/lib/libusb-1.0.so.0 (0x00007fbc8c0eb000)
	libhidapi-libusb.so.0 => /usr/local/lib/libhidapi-libusb.so.0 (0x00007fbc8c0e0000)
	libftdi.so.1 => /lib/x86_64-linux-gnu/libftdi.so.1 (0x00007fbc8c0d4000)
	libftdi1.so.2 => /usr/local/lib/libftdi1.so.2 (0x00007fbc8c0c3000)
	libedit.so.2 => /lib/x86_64-linux-gnu/libedit.so.2 (0x00007fbc8c08b000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fbc8be99000)
	/lib64/ld-linux-x86-64.so.2 (0x00007fbc8c38e000)
	libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x00007fbc8be7d000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007fbc8be5a000)
	libudev.so.1 => /lib/x86_64-linux-gnu/libudev.so.1 (0x00007fbc8be2b000)
	libtinfo.so.6 => /lib/x86_64-linux-gnu/libtinfo.so.6 (0x00007fbc8bdfb000)
	libbsd.so.0 => /lib/x86_64-linux-gnu/libbsd.so.0 (0x00007fbc8bde1000)

Edit to add: but there are no issues with libedit version of PR #1132 under Ubuntu 20.04. So this looks like an issue specic to libedit under macOS. I will check out the Homebrew version of libedit to see if that is okay or not.

Edit to add: unfortunately libedit Homebrew version has the same issue as the system version.

MCUdude · 2022-11-08T17:28:32Z

Sorry for bringing this up, but is libreadline now required for terminal to work properly on macOS?

I'm building using make, and when in terminal mode, I'll have to hit enter twice to get the avrdude> prompt.
When typing, the text appears on the line below.

There was never an issue with terminal mode before?

avrdude: AVR device initialized and ready to accept instructions
avrdude: device signature = 0x1e9705 (probably m1284p)
avrdude> ?
>>> ? 
Valid commands:
  dump    : dump <memory> [<addr> <len> | <addr> ... | <addr> | ...]
  read    : alias for dump
  write   : write <memory> <addr> <data>[,] {<data>[,]}
          : write <memory> <addr> <len> <data>[,] {<data>[,]} ...
  flush   : synchronise flash & EEPROM writes with the device
  abort   : abort flash & EEPROM writes (reset the r/w cache)
  erase   : perform a chip erase
  sig     : display device signature bytes
  part    : display the current part information
  send    : send a raw command: send <b1> <b2> <b3> <b4>
  parms   : display adjustable parameters
  vtarg   : set <V[target]>
  varef   : set <V[aref]>
  fosc    : set <oscillator frequency>
  sck     : set <SCK period>
  verbose : change verbosity
  quell   : set quell level for progress bars
  help    : show help message
  ?       : same as help
  quit    : quit after writing out cache for flash & EEPROM

Note that not all programmer derivatives support all commands. Flash and
EEPROM type memories are normally read and written using a cache via paged
read and write access; the cache is synchronised on quit or flush commands.
The part command displays valid memory types for use with dump and write.


avrdude> 
quit
avrdude> quit
>>> quit 

avrdude done.  Thank you.

mcuee · 2022-11-09T00:06:25Z

Sorry for bringing this up, but is libreadline now required for terminal to work properly on macOS?

I'm building using make, and when in terminal mode, I'll have to hit enter twice to get the avrdude> prompt. When typing, the text appears on the line below.

There was never an issue with terminal mode before?

Yes now libreadline is required. libedit is not compatible with #1132 unfortunately.

stefanrueger added the help wanted Extra attention is needed label Jul 15, 2022

mcuee added the enhancement New feature or request label Oct 16, 2022

stefanrueger mentioned this issue Oct 18, 2022

Revamp terminal output (progress bar, callback and stdout/stderr) #1132

Merged

stefanrueger linked a pull request Oct 20, 2022 that will close this issue

Revamp terminal output (progress bar, callback and stdout/stderr) #1132

Merged

stefanrueger closed this as completed in #1132 Oct 23, 2022

stefanrueger mentioned this issue Nov 8, 2022

Provide Urclock programmer #1171

Merged

stefanrueger mentioned this issue Nov 20, 2022

Avrdude on macOS is built without libreadline #1173

Closed

mcuee mentioned this issue Nov 21, 2022

Readline support for MSVC #1186

Closed

mcuee mentioned this issue Dec 16, 2022

PICkit4 and SNAP serial numbers not read by Avrdude #1038

Closed

Portable readline with callback required #1028

Portable readline with callback required #1028

Comments

stefanrueger commented Jul 15, 2022 • edited by mariusgreuel Loading

dl8dtl commented Jul 15, 2022

stefanrueger commented Jul 15, 2022

dl8dtl commented Jul 15, 2022

dl8dtl commented Jul 15, 2022 • edited Loading

dl8dtl commented Jul 15, 2022

mcuee commented Jul 16, 2022 • edited Loading

mcuee commented Jul 16, 2022 • edited Loading

mcuee commented Jul 16, 2022 • edited Loading

stefanrueger commented Jul 16, 2022

stefanrueger commented Jul 16, 2022 • edited Loading

mcuee commented Jul 16, 2022 • edited Loading

mcuee commented Jul 16, 2022

mcuee commented Jul 16, 2022

mcuee commented Jul 17, 2022 • edited Loading

dl8dtl commented Jul 17, 2022

mcuee commented Jul 17, 2022 • edited Loading

stefanrueger commented Jul 18, 2022

mcuee commented Jul 31, 2022 • edited Loading

mcuee commented Sep 5, 2022

mariusgreuel commented Sep 5, 2022

mcuee commented Sep 6, 2022 • edited Loading

stefanrueger commented Sep 6, 2022 • edited Loading

stefanrueger commented Sep 6, 2022

stefanrueger commented Sep 6, 2022

mcuee commented Sep 6, 2022 • edited Loading

mariusgreuel commented Sep 6, 2022

mariusgreuel commented Sep 6, 2022

mariusgreuel commented Sep 6, 2022

stefanrueger commented Sep 6, 2022 • edited Loading

mariusgreuel commented Sep 6, 2022

stefanrueger commented Sep 15, 2022

stefanrueger commented Sep 16, 2022

mariusgreuel commented Sep 17, 2022

mariusgreuel commented Sep 17, 2022

stefanrueger commented Sep 20, 2022

mariusgreuel commented Sep 21, 2022

stefanrueger commented Sep 21, 2022

stefanrueger commented Sep 23, 2022

mariusgreuel commented Oct 8, 2022

mcuee commented Oct 21, 2022 • edited Loading

stefanrueger commented Oct 21, 2022

mcuee commented Oct 22, 2022 • edited Loading

mcuee commented Oct 22, 2022

mcuee commented Oct 22, 2022 • edited Loading

MCUdude commented Nov 8, 2022

mcuee commented Nov 9, 2022

stefanrueger commented Jul 15, 2022 •

edited by mariusgreuel

Loading

dl8dtl commented Jul 15, 2022 •

edited

Loading

mcuee commented Jul 16, 2022 •

edited

Loading

mcuee commented Jul 16, 2022 •

edited

Loading

mcuee commented Jul 16, 2022 •

edited

Loading

stefanrueger commented Jul 16, 2022 •

edited

Loading

mcuee commented Jul 16, 2022 •

edited

Loading

mcuee commented Jul 17, 2022 •

edited

Loading

mcuee commented Jul 17, 2022 •

edited

Loading

mcuee commented Jul 31, 2022 •

edited

Loading

mcuee commented Sep 6, 2022 •

edited

Loading

stefanrueger commented Sep 6, 2022 •

edited

Loading

mcuee commented Sep 6, 2022 •

edited

Loading

stefanrueger commented Sep 6, 2022 •

edited

Loading

mcuee commented Oct 21, 2022 •

edited

Loading

mcuee commented Oct 22, 2022 •

edited

Loading

mcuee commented Oct 22, 2022 •

edited

Loading