LCDProc 0.5.4-dev

mdima

…
From you commit, it seems like this code already exists in the client, but specifically for the CFonts driver:
Maybe the logic for LED4 is what makes sense here...

If you want to implement the same algorithm for the CFontz driver, you can use values frin 0 to 255. Every bit of the byte turns on a led, as follows:
bit 0: first led green
bit 1: second led green
bit 2: third led green
bit 3: fourth led green
bit 4: fifth led red
bit 5: sixth led red
bit 6: seventh led red
bit 7: eight led red

if you combine bits, in CFontz, you get "orange" (even if I didn't use that).
The syntax is: output<space>value[0..255]

If you are developing the driver, maybe it could have sense to use the same code in order to have only 1 algorithm to mantain… also you can start with one led and add the others when it will be discovered how to control them...

What do you think about that?

Ciao,
Michele</space>

fmertz

@mdima:

I don't know if Watchguard is the only company that use that panel… maybe something like:
"sdeclcd (Watchguard Firebox w/ SDEC LCD, etc)"
would be better, what do you think?
The same could be for all the other panels... would be a little mess to keep them updated.

This driver is for the SDEC LCD (generic, I guess), but specifically coded for the Watchguard wiring (parallel port with this control bit going to this particular line on the LCD,…). Others could possibly have an SDEC LCD, but there are many possibilities of wiring it, even with a parallel port. Bottom line is that I would not expect this driver to work on any other implementation of an SDEC LCD without reworking some of the code. As far as I know, the only testing that has ever been done is with these Fireboxes. Also, as you know, these boxes came with the SDEC LCD built-in, there is no home-made setups this driver is supposed to support.

At some future point, when the current models of Fireboxes become retired and available in the used market, they will need another driver anyway. I was hoping they would all sort nice like:

Watchguard Firebox w/ SDEC LCD
Watchguard Firebox w/ <new>LCD

No worries, whatever works.</new>

fmertz

@mdima:

if you combine bits, in CFontz, you get "orange" (even if I didn't use that).

If you are developing the driver, maybe it could have sense to use the same code in order to have only 1 algorithm to mantain… also you can start with one led and add the others when it will be discovered how to control them...

That is the general idea, but there is no blink supported in there. I'll have to think of something.
There is no Orange supported on Fireboxes, as far as I know, and only one LED location visible to users.
Well, the actual control of the 2 LEDs is with GPIO. It boils down to simple "out" instructions, but coming up with code that determines the right ports/bits in a portable manner is a bit of a challenge. GPIO is part of the Southbridge device, and a number of different devices have been reported. And with each device, the LEDs were mapped to different bits. Stephenw10 has it all mapped, fortunately, so now it is a matter of coming up with clean code for it. Where would the fun be without a challenge!

stephenw10

Renaming the menu option as 'Firebox (sdeclcd)' would probably be useful. If only to reduce the number questions on the forum!
The keypad is a Watchguard part it's not on the sdeclcd module so in the unlikely event that someone was using a compatible sdec display it would need a different keypad driver anyway (if it had a keypad).

Steve

Brak

@mdima:

@Brak:

@mdima:

Hi Brak,
lcdexex is not run at all, the package use its own client to interact with LCDproc. Sorry about the question, why do you need it to run?

I use a menu I made that runs in LCDExec, it does like reboot commands and stuff. I can't get it to cleanly start up with this package.

Hi Brak,
did it work with the previous version of LCDproc? (I mean, not LCDproc-dev) Just to understand if it's something I made that interfers with your menu.

Can I have your lcdd.conf? Just to look which are the changes you need for your menu to work…

Thanks,
Michele

Well, tbh, I can't get the completely default package to work with the example menu if I rename "lcdexec.conf.sample" to "lcdexec.conf" - I checked perms, too. And yes, I had it working with that standalone firebox LCDProc package that was on here. All I did was execute "./lcdexec -c ./lcdexec.conf" after LCDProc was up and running.

Am I missing something to enable LCDExec and for it to run from the lcdexec.conf?

mdima

@Brak:

Well, tbh, I can't get the completely default package to work with the example menu if I rename "lcdexec.conf.sample" to "lcdexec.conf" - I checked perms, too. And yes, I had it working with that standalone firebox LCDProc package that was on here. All I did was execute "./lcdexec -c ./lcdd.conf" after LCDProc was up and running.
Am I missing something to enable LCDExec and for it to run from the lcdexec.conf?

well… I am updating LCDproc to version 0.5.5 (package 0.8, will be released soon), I hope that this will solve your problem. If not please send me the your file lcdd.conf that I try to look what is wrong. Consider that this file changes machine to machine and does not contain any sensitive information.

Thanks,
Michele

mdima

Hello,
I just pushed a new version for the package. The changes are:

Updates LCDProc to 0.5.5 (very experimental!)
Rename the sdelcd driver to "Watchguard Firebox w/ SDEC LCD"

All feedbacks are welcome!
Michele

stephenw10

Tested 0.8 on my X-peak box after upgrading it to 2.0.1. Seems to work OK. :)

A couple of small things I noticed, this may have been true of previous versions I'm not sure.

The screen does not appear to be scrolling. I have the uptime screen enabled and it displays "up 12mins, 1 us " where it should say 'user'. Though there are characters available to display that. Perhaps a driver problem?

The service does not start until I go to Status: Services: and click 'start' next to LCDproc. This seems odd to me as the service should be restarted every time the changes are made to the config by the call to 'LCDproc.sh restart'.

Edit: Reading through lcdproc.inc (for the hundredth time!) it will only call for a service restart if lcdproc is already running. If it is not running and changes are made, as is the situation immediately after installation, then there is no call to start it.

This is where I hit trouble trying to make the package firebox only. It's not obvious to me how the start and stop logic works. It looks like the only way to stop the service is to remove the settings rather than uncheck the 'enable LCDproc' tick box. :-\ As I said this is where my skills ran out so I could easily be missing something.

Also there appears to be no point calling for a service restart since start contains stop as it's first command. Calling service start would seem to be sufficient.

Steve

fmertz

@stephenw10:

The screen does not appear to be scrolling. I have the uptime screen enabled and it displays "up 12mins, 1 us " where it should say 'user'. Though there are characters available to display that. Perhaps a driver problem?

I compiled the driver code in the lcdproc-0.5.5 tree. The resulting file is
https://github.com/downloads/fmertz/sdeclcd/sdeclcd.so
Just in case there is an updated needed from lcdproc-0.5.4 …

Feel free to update the driver file itself and restart. If it works better, I'll be happy to open a request with pfSense to make it the default for everyone.

mdima

Hello Steve,
I try to quote you:

@stephenw10:

Tested 0.8 on my X-peak box after upgrading it to 2.0.1. Seems to work OK. :)

That's great, also on the "sureelect" driver seems to work OK!

@stephenw10:

A couple of small things I noticed, this may have been true of previous versions I'm not sure.
The screen does not appear to be scrolling. I have the uptime screen enabled and it displays "up 12mins, 1 us " where it should say 'user'. Though there are characters available to display that. Perhaps a driver problem?

and again, the same problem is with my driver, so it does not look like something driver related.
But the "widgets" are sent all as "scroller", so I don't know what this is related to… I need to investigate a bit, maybe it is something broken in the package, maybe it's something broken in the LCDproc binaries...

@stephenw10:

The service does not start until I go to Status: Services: and click 'start' next to LCDproc. This seems odd to me as the service should be restarted every time the changes are made to the config by the call to 'LCDproc.sh restart'.
Edit: Reading through lcdproc.inc (for the hundredth time!) it will only call for a service restart if lcdproc is already running. If it is not running and changes are made, as is the situation immediately after installation, then there is no call to start it.

well… when you start the box, the service is started (if it's enabled in the package configuration).
Then when you apply some changes to the service settings, the service is restarted. If the service is not running when the changes are applied to the package, the service is not started.
This was the behavour the package had before, and I find it reasonable (if I stopped the service for some reason, it just stay stopped).
Do you agree with that?

@stephenw10:

Also there appears to be no point calling for a service restart since start contains stop as it's first command. Calling service start would seem to be sufficient.

The point is that to apply the changes you make to the configuration, the service must be restarted. Some of the settings and all the "screens" are defined only when the service starts, so if for example you add a new screen, the service must be restarted to apply this change. But there's no need to stop or restart the service manually before applying any change.
I agree that this "stop-restart" is reduntand since in the function "start" there's already a "stop the service" command, this is a bit more "safe" and has been inherited by the previous developer… I think that the previous developer thought to make it safer since for this package there are more than one program running, and in this way he was really SURE that before a start the service was not running anymore. I could simplify it, but I would prefer to keep it "safer" as it is (anyway, if the service is not running when it's called a "start", no code is executed). Consider that the package can be run also from other parts, or from the interface, from a refresh, and so on, this "safe" code avoids to have more than one program running in the background!

Ciao,
Michele

mdima

Hi Steve,
about that:

@stephenw10:

The screen does not appear to be scrolling. I have the uptime screen enabled and it displays "up 12mins, 1 us " where it should say 'user'. Though there are characters available to display that. Perhaps a driver problem?

after some investigation I found out that is the "uptime screen" that has a little problem, not the scrolling. If you enable the "interface screen" you will see your panel scrolling… Anyway, I fix that!

Ciao,
Michele

stephenw10

@mdima:

This was the behaviour the package had before, and I find it reasonable (if I stopped the service for some reason, it just stay stopped).
Do you agree with that?

Yes that seems reasonable and expected behaviour.
However I think that most users would expect the service to start if you tick the 'enable LCDproc' check box and then click save.
This is especially true just after installation when you have first setup the variables.
I think either it should instruct users to start the service manually or the service should start automatically.
Perhaps relabel the check box 'Enable LCDproc at boot time'?

@mdima:

Consider that the package can be run also from other parts, or from the interface, from a refresh, and so on, this "safe" code avoids to have more than one program running in the background!

I agree. :)

Steve

mdima

Hi Steve,

@stephenw10:

Perhaps relabel the check box 'Enable LCDproc at boot time'?

yes, I agree… you will find it the next release!

Ciao,
Michele

stephenw10

For further reducing user confusion you can include instructions using the <after_install_info>tag in pkg_config.8.xml. E.g.


 <after_install_info>Now configure your display in Services: LCDProc: then start the daemon in Status: Services:</after_install_info>

or something like that! ;)

Steve</after_install_info>

mdima

@stephenw10:

For further reducing user confusion you can include instructions using the <after_install_info>tag in pkg_config.8.xml. E.g.
 <after_install_info>Now configure your display in Services: LCDProc: then start the daemon in Status: Services:</after_install_info> 
or something like that! ;)
Steve</after_install_info>

yes, that's totally a good idea… you'll find it in the next release...

just a question (to you and to all): when do you think that we could replace the lcdproc package with the new lcdproc-dev package? I mean, to release it for real?

Thanks,
Michele

stephenw10

I would suggest we run both in parallel at least until 2.1 is released. At that point the package system is changing so work would be required to the old package which seems pointless if the dev package is relatively bug free.

Perhaps ask Seth Mos(databeestje)?

Steve

mdima

@stephenw10:

I would suggest we run both in parallel at least until 2.1 is released. At that point the package system is changing so work would be required to the old package which seems pointless if the dev package is relatively bug free.
Perhaps ask Seth Mos(databeestje)?
Steve

ok… I know that we are adding a lot of new features, but the "basic" features of lcdproc seem to be quite stable (I am running the service since days and days, with 1 second as refresh and it never hangs, while lcdproc 0.5.3.1 was hanging only after few days with a 5 seconds refresh). Actually I have no need to release it, but it would be great if also the other users could take advantage of this version, even if I don't have any number about the people that are using the package and their feedbacks.

Btw, before starting to put some work on the package, I tried to contact Seth Mos but the email was returning... but of course I will contact again before releasing the package on the "main" version...

Michele

fmertz

@mdima:

just a question (to you and to all): when do you think that we could replace the lcdproc package with the new lcdproc-dev package?

What is this package released against? A production pfSense release, or a development pfSense release? If it is development, I would say release it soon. This way, you can catch problems from users of other drivers before the package becomes production…

stephenw10

It would be very interesting to know how many people are using it.
I don't think we can ever hope to test everything before declaring it 'ready'.

I'm not sure really. :-\

Steve

mdima

@fmertz:

@mdima:

just a question (to you and to all): when do you think that we could replace the lcdproc package with the new lcdproc-dev package?

What is this package released against? A production pfSense release, or a development pfSense release? If it is development, I would say release it soon. This way, you can catch problems from users of other drivers before the package becomes production…

Hi fmertz,
I ment to release in production (replace the "lcdproc" package with the "lcdproc-dev" package)…

opinions opinions! ;)

Thanks,
Michele