Re: [Simulavr-devel] [PATCH] Reduce warnings about registers not simulated

[Torsten Duwe]

On Thu, 23 Feb 2012, ThomasK wrote:

> Hi Torsten,
>
> thanks for your informations about opensuse package. A I can see, that you've 
> produced packages for 11.4 and 12.1. Is this then also included in opensuse 
> distribution, e.g. it's installable by yast or yum or such? I think, I'll add

opensuse.org hosts all those "projects", run by Suse employees,
contributors, or both. The installation media only hold the very base 
distribution; you choose from those projects the ones you want to add.

zypper ar 
http://download.opensuse.org/repositories/CrossToolchain:/avr/openSUSE_12.1 
AVR-12.1

will add this project to your e.g. openSuse-12.1, and if you trust those 
maintainers (accept the build key), you'll have CrossToolchain:avr as part 
of your system. The build service can also support other distros, BTW.

> a comment in documentation. And, as I've seen, this is only simulavr itself, 
> not python or tcl interface, right?

What do those consist of? I split off a simulavr-devel package that holds 
some stuff. You'll see the binary packages in the download repository.

> > I made 2 patches, the first of which I'd like you to consider for
> > upstream inclusion. Please comment or apply.
>
> 2 patches? You mean this patch on 2 files in your message or do I've overseen 
> the other patch? :-)

No, the other patch hits autoconf with a big hammer. I dislike 
autoconf/automake/libtool, and hack it only so far as to that it does what 
I want. It is not clean at all, and not suitable for inclusion, IMHO.

> > NotSimulatedRegister warns on stdout about each r/w access to each
> > instance. This causes not only the regular startup messages to scroll
> > off screen, but also warnings about config registers used only once
> > during device initialisation. This patch makes simulavr warn only once
> > for every register written, and for the 10 first registers'
> > read-before-write accesses.
>
> Good Idea. And you're right, because of some peripherals are not implemented 
> or if somebody tries to use a register, which isn't available, but it would 
> not influece the control flow, if used, it would produce a lot of warning 
> messages again and again. So it looks as a good idea to limit this messages. 
> (it's not only NotSimulatedRegister class, also InvalidMem class by example)
>
> On the other hand, maybe later in simulation, where it's necessary to get a 
> hint, why code isn't running as expected, it would not bring this message. So 
> it's a classic conflict, to much is bad, but to less also.
>
> I'm not sure, if a command line option could help to switch the behaviour 
> between all messages or only the first/first 10. Any ideas?

Take a closer look: write access to each unsimulated reg is reported 
always, and exactly once; that's what I introduced the flag for. The read 
access via get() is const, unfortunately, and allows no alteration of 
"*this". I had a much more intrusive patch, which removed the const all 
through the class hierarchy. That would be "the right way", but probably 
has some performance impact. Then I thought about using a flag only for 
set(), and simply cut off logging invalid get()s globaly at some 
threshold. How many registers are there that are constantly read, but 
never written to?

My motivation was that my test app uses an MCU sleep mode, and simulavr 
spilled stdout with "MCU register SMCR not simulated". With this patch, 
I'm now aware that TWI isn't simulated either :) It does a warning no good 
to repeat it millions of times.

Would you prefer to remove the const from the get() method?

        Torsten