Re: [hatari-devel] Recent Hatari change problems

[Thomas Huth <th.huth@xxxxxxxxx>]

Am Fri, 13 Apr 2018 01:43:42 +0300
schrieb Eero Tamminen <oak@xxxxxxxxxxxxxx>:

> Hi,
> 
> On 04/12/2018 01:05 PM, Thomas Huth wrote:
> > On 11.04.2018 22:32, Eero Tamminen wrote:  
> >> On 04/11/2018 11:08 AM, Thomas Huth wrote:  
> >>> On 11.04.2018 01:23, Eero Tamminen wrote:  
> [...]
> >> E.g. changing option output to happen only with the lowest "debug"
> >> level is just broken.  It means that user doesn't get any feedback
> >> when he changes something in debugger.  Please fix that.  
> > 
> > Honestly, I consider these messages as *debug* messages. So I'm not
> > convinced that we should spam the output with these messages.
>  > At least not while parsing the command line option.  
> 
> What's the problem you're solving?

See my other mail about "make test" - to be able to "diff" the output
of Hatari, I need a way to shut up all non-related messages.

> When user manually adds an option to Hatari command line, I consider
> it interaction, and it's very nice to get feedback for that.

Sorry, but that's nuts. If you run "ls -l" on the command line, do you
also expect ls to print out something like "Running in long listing
format mode" ?? Certainly not. The users just typed in the command line
parameter, so they know what they typed there and there is really no
good reason for printing out that stuff each and every time again
(unless you're debugging the behaviour of Hatari, and that's what
the debug log level is good for).

> Other use-case for command line options is wrappen scripts, and often
> in that case user doesn't even see Hatari output, so few extra lines
> shouldn't matter.

In that case the users either don't care about the options, or if they
do they can simply look them up in the script. Again no need to spam
the console output with this.

I have to say that I'm normally rather annoyed if programs spam my
console window with debug messages unless I've told them to do so.

> Btw. I just noticed that you had hidden also CPU core warnings
> e.g. about things like illegal instructions.  Why?
> 
> IMHO it's important information when something is failing, but user
> is very unlikely to find out that he needs to add "--log-level debug"
> to get Hatari now to tell about CPU related problems.

I think the average user of Hatari even does not know what an illegal
instruction message means - and in some cases these are also part of
the normal operation and not a problem. Analyzing the output of the CPU
core normally requires at least basic understanding of the 68k CPUs
already, so we really rather should not present that stuff to the
normal users by default.

Well, even if we decide that the "important" messages should be shown
by default, the main problem here is that the UAE core does not know
about log levels, or rather differences in importance of messages. It
just writes out all kind of messages with one function (write_log()), no
matter whether it's rather a debug message or rather a emulation-error
related message. So it's all-or-nothing for the messages here. I
consider most of the messages from the CPU core rather to be debug
messages, so mapping all of them to LOG_DEBUG sounds like the right
level to me here. But I might be convinced that we could also increase
the log level of these messages to LOG_INFO instead - if others think
that this is useful. Nicolas, Laurent, others on the list, what do you
think?

> Same as with Debugger, interactive use of Hconsole requires
> feedback/acknowledgement on what user requests.

That's bad design if this is really required here. If Hatari is
remote-controlled, it e.g. can not even know whether stderr is available
for printing or not. If you really needed to pass a result string to
the calling program, it should be passed via the same channel as the
commands, i.e. via the control socket instead.

> >> Btw. Why the commit to make XBios test "more reliable", changes
> >> Hatari to be run with "--cpuclock 32" instead of using
> >> "--fast-forward on"?  
> > 
> > The basic idea is to "test" more command line parameters.
> > "--fast-forward" is already tested by the bus error tester, so I
> > wanted to check "--cpuclock" in one of the testers, too. The test
> > runs reasonable fast anyway so that you won't notice a big
> > difference here.  
> 
> Shouldn't test for "--cpuclock 32" check that CPU actually works
> faster than 16Mhz?

The idea here was rather to check whether Hatari can execute any code
with --cpuclock 32 at all, or whether the option has been broken by
accident.

For a "proper" CPU clock test, we'd need a test to check the amount of
cycles needed for certain instructions, of course.
(I think I still got some assembly code for this on my HD somewhere, so
when I've got a lot of spare time, I might be able to include that in
the regression test suite...)

> >> * "--fastfdc yes" when tests don't use floppy  
> > 
> > No test is using the FDC yet, so there should not be any difference
> > here.  
> 
> At least with real TOS, that option has definite impact on
> startup speed, regardless of test program actually uses
> FDC (because of the TOS floppy test timeouts).

Sure, but no TOS also means no floppy test timeouts :-)

> >> * "--fast-boot yes" when TOS memory detection isn't needed
> >> * "--timer-d yes" when tests don't depend on timer-D interval  
> > 
> > Tests run without TOS, so I'd also not expect any difference with
> > these.  
> 
> Right, I forgot that.
> 
> Is that supposed to work now?  For example if I run:
>    ./src/hatari  --tos none  tests/xbios/xbiostst.prg
> 
> Or anything else, only thing I get right away is double
> bus error & debugger halt.

You also need to specify "-d tests/xbios/" to enable GEMDOS HD
emulation and for this test, you also need "--bios-intercept on" of
course.

> >> I would also recommend "--mousewarp off" to avoid Videl code
> >> "randomly" moving mouse on screen when emulation is booted.  
> > 
> > Again: Test run without TOS, so no mouse involved here. I'll
> > summarize this in my mail about "make test" on the next weekend.  
> 
> Without "--mousewarp off" option, Hatari centers the mouse to its
> window on startup, even if one uses "SDL_AUDIODRIVER=dummy" and
> "--disable-video yes".
> 
> With Videl, the mouse move happens additionally on every
> Atari resolution change.
> 
> Both can be annoying if one is doing something else while
> these tests run.

Again, the tests are not run with a window (also not with
"--disable-video yes"), but with SDL_VIDEODRIVER=dummy. In this
mode, SDL simply ignores all UI related things, so mouse or
resolution changes simply do not matter.

> >> IMHO the old C version was  more readable than the assembly
> >> one.  This API is so rarely used, that I don't think it matters
> >> though. :-)  
> > 
> > I did this rewrite to assembler due to the limitations of the "--tos
> > none" mode (and since I did not want to put larger binaries into the
> > repository).  
> 
> xbiostst.prg is 296 bytes, while dbmsg.tos was 3750 bytes.
> 
> Is couple of KB really a large binary???

I think everything below 10kB is fine for inclusion into the Hatari
repository. I'm just a little bit afraid that with C code, people will
then be tempted to include binaries that have been compiled with GCC +
mintlib. And these are huge, e.g.:

$ echo 'main(){}' > tst.c
$ m68k-atari-mint-gcc tst.c -o tst.prg
$ ls -l tst.prg
-rwxrwxr-x. 1 thomas thomas 230901 14. Apr 15:17 tst.prg
$ m68k-atari-mint-strip tst.prg
$ ls -l tst.prg
-rwxrwxr-x. 1 thomas thomas 117730 14. Apr 15:17 tst.prg

The problem with C is the startup code - it can be very large and you
don't know which OS functions it depends on - and we only have a very
small amount of GEMDOS functions available in the "--tos none" mode.

> Regarding the NatFeats tests...
> 
> * You could add something like "--run-vbls 5000" to make sure
>    the Hatari exits in case NatFeats for some reason didn't get
>    enabled.  Or does CTest already have some timeout for tests?

The natfeats test already uses --run-vbls, see the run_test.sh script
there.

ctest also seems to have a "--test-timeout" option, but the man page
says that it is "internal only".

> * the nf_vbcc.tos binary in the repo is old build, it's missing
>    debugger invocation in the latest natfeats.c version.
>    Which would make it unsuitable for automation, as there's
>    currently no way to automatically continue emulation from
>    debugger, it requires user interaction.

I noticed that the binary does not quite match the C file anymore, so
at one point in time, we should indeed update it...
Debugger invokation is not a real problem, that happens in the xbios
Dbmsg test, too. We just have to feed some "c" characters into the
stdin of Hatari - see tests/xbios/run_test.sh for an example.

> * Latest AHCC NatFeats build is about same size (<5KB). Compared
>    to VBCC build, it does at start several Fseek()s to special
>    conin, conout, aux, prn & bioscon handles, with successive
>    offsets of 0, 1 and 0. Not sure why.
> 
> -> Not sure whether I should update the binaries or not.  
> 
> I guess TOS "none" could ignore all Fseek()s to special handles
> (0-3 and 0xFFFB-0xFFFF)?

The cleaner way is maybe to rather provide our own startup code for C,
which just does an Mshrink and then immediately jumps to main(). That
way we could produce real small binaries and can be sure that no
non-supported OS calls are being called by the startup code.

 Thomas