Rockbox

Tasklist

FS#1897 - Win32sim 2.1 Update (build 8)

Attached to Project: Rockbox
Opened by Blue Chip (bluechip) - Friday, 26 December 2003, 05:34 GMT
Last edited by Daniel Stenberg (bagder) - Monday, 10 May 2004, 15:12 GMT
Task Type Patches
Category Simulator
Status Closed
Assigned To Daniel Stenberg (bagder)
Operating System
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No

Details

What is fixed?
""""""""""""""
o Build support for ALL Archos players
o MAS-3587 specific features
o Radio specific features
o .rock Plugin support
o .rockbox/*.CFG support
o .rockbox/*.WPS support
o .rockbox/*.FNT support
o .rockbox/*.LNG support


What is NOT fixed?
""""""""""""""""""
o Archos _Player_ screen display seems wrong to me.
o Both of these plugins fail with
"Can't find entry point"
o RVF Video
o CH8 Chip8
o To play Sokoban you will need to create
.rockbox/sokoban and place the levels.txt file
in it.


New .rockbox Directory Support
""""""""""""""""""""""""""""""
o The "archos" directory is now called ".rockbox" for
logical reasons.
o *.rock files and NOT compiled to ".rockbox/rocks" but
instead, to the build directory.
...this is ONLY for simulation purposes


Browser Root
""""""""""""
Browser root is the root directory of the Windows drive
on which cygwin is installed.

==PLEASE==
Report ALL successes and failures with this patch :)

BC
This task depends upon

Closed by  Daniel Stenberg (bagder)
Monday, 10 May 2004, 15:12 GMT
Reason for closing:  Out of Date
Additional comments about closing:  Logged In: YES
user_id=1110

There's no patch here, there's nothing provided since my
latest feedback.
Comment by Eric Linenberg (elinenbe) - Friday, 26 December 2003, 23:18 GMT

nice patch. keep up the great work!
Comment by Daniel Stenberg (bagder) - Thursday, 01 January 2004, 17:07 GMT

Over all, this is good work. I have some comments/remarks:

1. The plugin_load() fix could be made to find files in a
different place, just like open() is fixed.

2. sprintf(buf, file->name); is a bad idea, as file names
*can* contain %d or %s etc.

3. I don't like the #ifdef SIMULATOR lines in the config-*.h
files.

4. In many places you've added #ifdef SIMULATOR ... #else
... #endif for several functions. Perhaps you could make one
big #ifdef instead, even possibly putting the stubs in one
of the simulator-unique source files.

5. Regarding the configure patch, we already set the TARGET
variable to @TARGET@, no need to set DEVICE too...

6. I don't like the rename of the root dir to .rockbox from
archos.
Comment by Blue Chip (bluechip) - Friday, 02 January 2004, 04:17 GMT

>Over all, this is good work.

Thanks

>> 1. The plugin_load() fix could be made to find files in a
different place, just like open() is fixed.

I have done this all very differently in the latest build ...take
another look, and maybe comment again?

>> 2. sprintf(buf, file->name); is a bad idea, as file names
*can* contain %d or %s etc.

Yes, a sloppy mistake by me - gone! :)

>> 3. I don't like the #ifdef SIMULATOR lines in the config-*.h
files.

It was the KISS solution.
I don't really understand why you dislike it ...I am open to
better ideas.

>> 4. In many places you've added #ifdef SIMULATOR ... #else
... #endif for several functions. Perhaps you could make one
big #ifdef instead, even possibly putting the stubs in one of
the simulator-unique source files.

The coding-style is VERY "patchy" on this ...some procs are in
uisim/common/stubs.c ...some procs have the main body of
the code wrapped in #ifndef SIMULATOR ...I felt that my idea
was much like the second system, but MUCH easier to read
and edit.

I do not like the stubs idea (for rockbox) ...I believe my
system means that mpeg.c (for example) contains ALL the
mpeg routines ...no matter if you compile sim or ajz ...using a
stubs file means that those who work ONLY on sim/ajz might
miss required changes in the other and make a broken patch.

_Personally_, I do not like giant #ifdefs ...they are difficult to
spot especially when they are nested with many other #ifdefs
...and we do not indent for preprocessor directives ...the
#ifdef on line 2360 of the new mpeg.c is fun to pair-up!
...but, although I do not like it, I suppose it is a coding style,
so I will be interested to hear what others have to say before
commiting myself one way or the other.

I realise that there are a lot of places that the code-style
could be more consistent ...I would be happy to go through
and add consistency to the code, and I am sure we could all
agree on what we liked (after much debate - lol)

>> 5. Regarding the configure patch, we already set the
TARGET variable to @TARGET@, no need to set DEVICE too...

TARGET gets destroyed by uisim/win32/makefile ...that is the
ONLY reason I added DEVICE

I am really rather bad at following configure & makefile's ...If
you are good at that stuff, I encourage you to change this
problem in any way you see fit :)

...I was just pleased to get it to work - LOL

>> 6. I don't like the rename of the root dir to .rockbox from
archos.

No, several comments have been made on this. The old
win32sim had a bug with the root directory and I fixed it ...in
the wrong way.

The new solution is to use the tree ~/source/build/archos/.
rockbox/rocks
The UISIM.EXE will then look in ./archos for it's root directory

Linus mentioned about having ./player and ./recorder (instead
of ./archos) ...but having given it more thought, I believe a
better system would be ~/source/build-fm/archos and
~source/build-rec/archos ...having both builds in a single
build/ directory seems illogical to me ...I shall chat futher to
Linus on this when next I see him online :)

You may also create ./archos as a symbolic-link to wherever
you liked ...you would need to do this BEFORE you ran
configure!

Thanks for the feedback

BC
Comment by Blue Chip (bluechip) - Friday, 02 January 2004, 04:25 GMT

Build 8...

# Update patch generally ...to work with recent cvs

# Re-did the plugin simulation code

# Reinstate ./archos directory with .rockbox as a subdir
./archos may be created as a symbolic link
if desired by the user BEFORE running 'configure'

# Set broswer root ./archos

# FM Radio settings
use ROCKBOX_DIR not hard-coded directory

# Simulation patches for the following plugins:

# Calendar
use ROCKBOX_DIR not hard-coded directory

# Star
use ROCKBOX_DIR not hard-coded directory
Create directory entry in configure

# Sokoban
((use ROCKBOX_DIR not hard-coded directory))
Create directory entry in configure

# Clock
use ROCKBOX_DIR not hard-coded directory
Create directory entry in configure
** clock.c is in M$ format ...the patch will
fail and needs to be done by hand!
Comment by Daniel Stenberg (bagder) - Wednesday, 07 January 2004, 16:26 GMT

GAH!

darn crappy sorceforge went down and ate my huge comment
here... :-/ I'll rewrite and resubmit tomorrow or so.
Comment by Daniel Stenberg (bagder) - Thursday, 08 January 2004, 09:31 GMT

This work is even better now. I do have a few remarks, and
the following should not be taken as criticism but mostly
describes how to adjust this patch to be more in the spirit
of the simulator I envision we should be moving towards.

* The mpeg.c changes don't apply due to moved functions

* I don't like the special #define of ROCKBOX_DIR for the
simulator, as I want code that compiles and runs on both
target and simulator to use the same strings/paths and as
much code as possible should be common for both the
simulator and target (to keep string sizes equal and make it
easier to dump/view contents of variables when debugging
etc). Any simulator- adjustments of the path should be made
in simulator-specific functions. This also then concerns how
you change the call to dirbrowse() in tree.c. It should
remain browsing "/", but the simulator code should make that
equivalent of the "archos" subdirectory.

* I suggest that you make a simulator stub for the
mas_codec_writereg() function, as that will then remove the
need for #ifdef'ing the use of it. The same goes for
dac_line_in() and ata_poweroff() I think. Reducing the
amount of "#ifdef SIMULATOR" to a minimum is a good thing,
IMHO. That's also why I personally subscribe to the idea of
as far as possible using simulator-stubs put in separate
source files.

* You don't need to change the plugin_load() calls with the
PLUGIN_DIR define, as we're about to cleanup the plugins
paths and create a dir hierarchy anyway, so your changing of
those lines might collide with that separate cleanup-patch.
(This is of course not your fault.)

* I don't think those #ifdefs belong in the config-*.h
files, because those files define how the different models
work. The simulator simulates those models. I don't see why
NEED_ATA_POWER_BATT_MEASURE or BATTERY_SCALE_FACTOR
can't be defined in there even when the simulator is built.

* Request: can you make the additional directories you
create in the configure script into a list of directories to
create and then loop through that list? It would make it a
lot easier to maintain/edit that list as we proceed and
update the dir hierarchy in the future.

Thanks for your hard work on this.
Comment by Daniel Stenberg (bagder) - Thursday, 08 January 2004, 09:35 GMT

Uh, one more suggestion:

Can you make the copying of the rocks in the simulator
makefile only made on 'make install' ? I often redirect my
'archos' dir to point to an actual directory of mp3 files
and I would hate to get that filled with simulator-rocks
just because I decide to rebuild the sim...
Comment by Blue Chip (bluechip) - Monday, 12 January 2004, 07:32 GMT

Hi,

Badger, thanks for taking the time to supply me with some
good feedback.

Depressingly, the current requests lead to (what I think will
be) quite a lot of "makefile" and "configure" modifications.
Although my C coding is excellent, the complex use of
daisy-chained makefiles and use of BASH script are beyond me
:( ...hence the childish approach to the current
makefile/configure changes in build 8.

FWIW I generally agree with your [Badger] ideas. I did pen a
full reply (offline) but when I realised that I cannot complete
the work, it seemed irrelevant.

I am eager to do as much as I am able to get this working ...
So if anyone with sound makefile and BASH experience would
like to work with me to revive the, now, long-dead simulator
support PLEASE CONTACT ME ...if not, I fear that this part of
the project dies here :((

BC

P.S. As simulation of both 3587 Audio Settings and Plugins
does not work, this will also mean the death of the new GUI
3587 Audio Settings Plugin ...3000+ lines of untestable code :
(((
Comment by Daniel Stenberg (bagder) - Monday, 12 January 2004, 07:43 GMT

So leave as much as possible of the makefile and configure
stuff out of your patch, and allow me to fix those parts
after the rest of your patch goes in...
Comment by Blue Chip (bluechip) - Tuesday, 13 January 2004, 04:23 GMT

>>>leave as much as [necessary] of the makefile and
configure stuff out of your patch, and allow me to fix those
parts after the rest of your patch goes in...

Will do, thanks.
___________________
>>>The mpeg.c changes don't apply due to moved functions

Do we have any idea what is happening with this file ...(when)
is it likely to become stable again? ...it's a b-i-g boring change
to keep doing again and again.
___________________
>>>I don't like the special #define of ROCKBOX_DIR for the
simulator, as I want code that compiles and runs on both
target and simulator to use the same strings/paths and as
much code as possible should be common for both the
simulator and target (to keep string sizes equal and make it
easier to dump/view contents of variables when debugging
etc). Any simulator- adjustments of the path should be made
in simulator-specific functions. This also then concerns how
you change the call to dirbrowse() in tree.c. It should remain
browsing "/", but the simulator code should make that
equivalent of the "archos" subdirectory.

I presume your updates to the plugin system will work in
simulator mode, so I will look at how you achieve this with the
plugins and apply the same method to the browser, fonts,
languages, etc. etc.
___________________
>>>You don't need to change the plugin_load() calls with the
PLUGIN_DIR define, as we're about to cleanup the plugins

Okay - that can be removed easily enough
___________________
>>>PS. copy the rocks in the simulator makefile only made on
'make install'. I often redirect my 'archos' dir to point to an
actual directory of mp3 files and I would hate to get that filled
with simulator-rocks just because I decide to rebuild the sim...

1) Your directory would not get any rocks in it, just a rocks
directory.
2) sim:plugin_load() will need to know if "INSTALL" was passed
to the makefile so that it knows where to find it's plugins and
is able to frig the path at the last minute.
3) I think this would be a better idea...

void plugin_load(file, prams)
{
#ifdef SIMULATOR
#ifdef INSTALL
strcat(file,"./archos",file);
#else
strip_path(file);
#endif
#endif
int f = fopen(file,O_RD);
...
...
}
___________________
>>>I suggest that you make a simulator stub for the
mas_codec_writereg() dac_line_in() and ata_poweroff()
functions. Reducing the amount of "#ifdef SIMULATOR" to a
minimum is a good thing, IMHO.

Yes, and yes.
___________________
>>>I don't think those #ifdefs belong in the config-*.h files,
because those files define how the different models work.

I will look at other possible solutions
___________________
>>>additional directories in the configure script a list of
directories and then loop through them

The honest answer is no ...I can't ...I honestly wouldn't know
how ...I could write DOS BATch file to do it! ...but yes, it is an
MUCH better idea :)
___________________
>>>Thanks for your hard work on this.

Cool :) Glad it's worth the time :)
Comment by Daniel Stenberg (bagder) - Monday, 10 May 2004, 15:12 GMT

There's no patch here, there's nothing provided since my
latest feedback.

Loading...