| Re: [hatari-devel] Recent Hatari change problems |
[ Thread Index |
Date Index
| More lists.tuxfamily.org/hatari-devel Archives
]
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?
When user manually adds an option to Hatari command line, I consider
it interaction, and it's very nice to get feedback for that.
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.
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.
Got to ponder a little bit on whether it makes sense to always
print these messages during the debugger session or not.
Btw. Hatari command line and debugger are not the only things
which call option parsing. There are also:
- Hconsole terminal application
(both interactive use and scripts using it)
- Hatari python-ui
- XBios(255)
Same as with Debugger, interactive use of Hconsole requires
feedback/acknowledgement on what user requests.
In some cases it's pretty important information:
setopt --disasm 0xf
Changed CPU disassembly output flags from 15 to 0.
setopt --disasm 15
Changed CPU disassembly output flags from 0 to 15.
With your change, user wouldn't know that given option doesn't accept
hex values for the bitmask, but interprets them as decimals. I.e.
options output should be at least the same as Hatari default
log level.
Well, in that case, there should be an *error* message instead if there
there are unparsable characters in the input.
Fixed.
Anyway, "toggling" is a bad idea here, since the users first have to
change the option to know in which state it was.
It's off by default and doesn't have config option, because having
it enabled by default would be a security issue.
Sure, but still, the user has to remember the current state of the
option if it is just toggling the state. It's way more useful if you can
directly say whether you want to switch it on or off.
Sounds good to me, but it's still an API change which needs to be
documented. :-)
I've now changed it to a proper "boolean" option
("--bios-intercept on|off") which should be
more straight forward and fix this problem.
That breaks any programs that were using the above XBios() call.
Honestly, do you know any such program?
The point is that we wouldn't know, but I think it unlikely that
normal use-cases for this would involve untrusted code. AFAIK
people use XBios(255) mostly to speed up certain parts of their
own Atari code, when testing new builds of them.
If such changes are deemed necessary, they must be listed in
the release notes (maybe under "API changes" heading).
When updating command line options, please update both the Hatari
HTML manual and manual page, not just one of them.
Yeah, right, I guess I shouldn't rush patches during short breaks in my
busy week. I'll fix it at the weekend.
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?
FYI: Some other options that can be used to make emulation faster
in addition to "--fast-forward yes" and "--sound off":
* "--borders off" when overscan effects don't need to be visible
* "--spec512 0" when Spec512 effects don't need to be visible
* "--zoom 1" to lessen Hatari screen conversion overhead
* "--disable-video yes" when display output isn't needed
The tests are run with SDL_VIDEODRIVER=dummy anyway, so I'd not expect
much speed improvements for these options.
OK.
But if you see an improvement with one of these options, feel free
to add them to the run_tests.sh script.
* "--dsp off" when tests don't use DSP (for Falcon)
No test is using --machine falcon yet.
* "--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).
* "--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.
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.
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???
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 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.
* 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)?
- Eero