Rockbox

  • Status Closed
  • Percent Complete
    0%
  • Task Type Patches
  • Category User Interface → Simulator
  • Assigned To
    bagder
  • Operating System
  • Severity Low
  • Priority Very Low
  • Reported Version
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by bluechip - 2003-12-26
Last edited by bagder - 2004-05-10

FS#1897 - Win32sim 2.1 Update (build 8)

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

Closed by  bagder
2004-05-10 15:12
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.

nice patch. keep up the great work!

Project Manager

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.

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

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!
Project Manager

GAH!

darn crappy sorceforge went down and ate my huge comment
here… :-/ I’ll rewrite and resubmit tomorrow or so.

Project Manager

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.

Project Manager

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…

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 :
(((

Project Manager

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…

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 :)

Project Manager

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

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing