Rockbox

  • Status Unconfirmed
  • Percent Complete
    0%
  • Task Type Patches
  • Category Plugins
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Version 3.1
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by dataangel - 2009-02-02

FS#9861 - Plugin for flash card memorizing: mnemosyne

This is a new plugin that mimics the open source PC program mnemosyne (http://www.mnemosyne-proj.org/), a program that assists with memorizing flash cards. The plugin is fully functional, but it still needs a menu and assumes that the user has setup the folders it needs for storing cards before running (if you don’t create them it will crash). I’ll fix these issues in the near future, but in the mean time any feedback is appreciated :)

I should add that flash cards have to be authored on the computer, not in the plugin. They're stored as text files and the format is described in the comment at the top of the source file.

Diff here :)

mud commented on 2009-02-20 13:28

It seems like an interesting plugin, but it needs some work. Here are some comments:
- You have tabs in the source file, they should be replaced by 4 spaces.
- Some lines are too long, they should be less than 80 characters. (see docs/CONTRIBUTING)
- I've been told repeatedly that you shouldn't use floating point math in Rockbox. It appears the the math could be changed to fixed-point instead without too much trouble.
- The buttons are hard-coded and won't work on all targets. If it were me, I'd change the control system a bit to be more like selecting a number from a list, and then change it to use get_action instead of manually doing the controls like that. That way, you wouldn't have to come up with keypads for every target.
- It's in the games category. I would have guesses it was an application myself, but I'm not sure.
- The plugin segfaults in the simulator if one of its directories isn't found. Gracefully failing somehow instead would be better of course.
- I added a few flashcards and then ran through them once. After that it stopped showing me any cards though. Is it supposed to do that, or if there are no priority ones is it supposed to show me the old ones again indefinitely?

Thanks. This plugin is raw unfinished but so much useful to me.
The architecture is a little weird however, should be simplified before it gets more complex (I might work on this).
The plugin falls into an infinite loop when it runs out of card, and there is no way to get out. I fixed this issue making a little change in the function find_next_item(), but I let you the honor of checking it out before making the patch (I'm new to open source contribution, tell me if I'm doing wrong).
The modified function goes this way:

static bool find_next_item()
{

while(find_item_file()) {
	if(want_to_test_item())
		return true;
	if(cur_item_file > 0)
		rb->close(cur_item_file);
}
simple_display_text(seek_mode_name());
long but;
do {
	but = rb->button_get(true);
      }
while((but != BUTTON_SELECT) && (but != MNEMO_QUIT));
cycle_seek_mode();
init_database();
return (but == MNEMO_QUIT) ? false : find_next_item();

}

cheers!

I'll look at your patch Tuesday once my apartment has internet access setup ;) What do you find weird about the architecture? I'm more accustom to OOP style C++ so my plain C might be a little weird.

By storing something 50 times bigger than easy_factor, long int would do the job great, not even fixnum neaded, and the line of code just before could be stripped off

new_easy_factor = old_easy_factor + 
	(0.1 - (float)(5 - response_quality) * 
	 (0.08 + (float)(5 - response_quality) * 0.02));

(as a mathematicist, I personnaly think we would really benifit from stopping having fix or float as processor instructions (cheeper processors or more polyvalent parallelism) making non ints a matter of library or higher level language feature (more reliability, only calculating what you really need to process, taking care better of uncertainities), but this is getting out of topic :-) )

As for my correction of find_next_item(), it's just a very quick and dirty hack, badly mixing user interface and data processing, what we really need is the function to return false when there is no next item. It's that function whose architecture is a little weird. Well I do have great respect for functionnal programming, but I think we should only introduce that paradigm in c code when it makes the program simpler.

For the rest it's rather a matter of ornementation than architecture:
Stricktly using bool instead of float when it's that semantic which is being used (while(true) and not while(1) etc)
Always using the ++ operator on separate lines would make the program closer to rockbox guidelines (still c++, but so much cute than +1), and always using postposition (it wont change anything on separate lines)
A break instead of a goto in the plugin_start whould be, I think more readable, and symetrical to the continue statement, but maybe I'm just being emotive with gotos.

I do admit I'm making a lot of fuss for simple details (anyways you know my background now), but I think part of the joy of non commercial coding is thaking the time to make perfect code.

I might come soon with my concrete corrections soon.

ERRATUM
"Stricktly using bool instead of float " I meant int of course.
'thaking ' meant taking (oups, damn second languages)

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing