LCDproc development and user support list

Text archives Help


[Lcdproc] [Patch] Update contrib/patches/lcdprocserverOPT.diff


Chronological Thread 
  • From: peter AT adpm.de (Peter Marschall)
  • Subject: [Lcdproc] [Patch] Update contrib/patches/lcdprocserverOPT.diff
  • Date: Wed Feb 28 13:35:01 2007

Hi Peter,

On Monday, 26. February 2007 19:28, Peter McCurdy wrote:
> I needed to use the contrib/patches/lcdprocserverOPT.diff patch for a
> project I'm working on, and I've made a few fixes to it. Summary of
> the changes:
>
> * Now applies to lcdproc-0.5.1.
> * The patch was created from outside the lcdproc directory, making it
> easier to apply.
> * CFontz633 boot messages can use both lines of the display: if you
> specify a message longer than 16 characters, the second line gets set
> to characters 17+.
> * Now doesn't display empty menus - if you disabled the default LCDd
> server menu (one of the features of the patch), you could still enter
> the menus, but it would crash if you pressed the right arrow. It also
> looked pretty funny. I figured the safest and easiest thing to do was
> just to not display a useless menu.
> * Let the server menu name be configurable in the config file with the
> [menu]menuentryname="Menu Name" option in LCDd.conf.

Thanks for the patch to the patch ;-)
I cmmitted it to CVS (in both branches: MAIN and 0.5-stable).
[I updated the patch file, but I did not apply the patch itself]

> Hopefully this all makes sense. In fact, is there any reason why this
> whole patch shouldn't be applied to the main LCDproc tree? I've
> tested that it works well, and it doesn't change any default
> behaviour.

There are a few reasons why I did not apply the patch:
1) It's a combination patch that tries to cover various
unrelated issues: startup message for a specific LCD,
changes to the menu, shutdown message.
This mixture makes it harder to check what the patch really does.
2) At least one part of the patch belongs IMHO better
into a separate program and not into LCDd:
The startup message for the CFontz633 driver.
IIRC the startup message for CF-63[135] displays
gets stored in the LCD once it is set.
I.e. it does not need to be set each time.
Therefore a separate tool, which can be packaged with LCDproc)
is IMHO more aappropriate than the LCDd server.
3) the startup message for CFontz633 is specific to this LCD.
No other displays can make use of it. Therefore it should not
go into LCDd. (but it can go into a separate tool)
4) ". .. but it would crash if you pressed the right arrow. It also
looked pretty funny."
This explains it quite well.
The patch was a hack of the "works for me" type and the task of
cleaning it up to make it useful for distribution was not done.
5) documentation quality:
The only documentation regarding the patches are a few
sentences in mails. If you want to know what the patch does,
you have to read the code.
The relevant parts of the documentation (docs/lcdproc-user/...)
or the config file (LCDd.conf) are not updated

But I am not totally opposed to the patch (or parts of it).
If you split the patch into its components, add documentation,
and continue polishing it, I will have a much closer look at it.

> Here's a diff of my changes. Unfortunately, this is a diff of changes
> to a diff file, which is a bit difficult to read, but interdiff gets
> confused by the changed level of the patch.

no problem: the diff applied ablolutely cleanly.

Thanks for helping LCDproc
Peter

--
Peter Marschall
peter AT adpm.de




Archive powered by MHonArc 2.6.18.

Top of page