dm140: add driver #47

awiouy · 2017-01-11T08:10:11Z

Hello,
The dm140 driver exists as a patch to LibreELEC/OpenELEC.
It would however be profitable to merge it upstream.
Thank you for your attention and support.

awiouy · 2017-01-11T08:16:13Z

Hm... It builds with LibreELEC.
I would need some help to fix failing tests.

haraldg

Thanks for looking into this. However clearly more work is needed before merging this (if possible at all). Please see the lcdproc developers guide for requirements of new drivers, especially copyright, documentation and coding style.

The various files included make me a bit uneasy. Where are the coming from? Shouldn't there be an external dependency instead?

I added a number of inline comments, pointing out obvious issues. But this is far from complete.

Also it seems your code doesn't acutally build. Have a look at the build logs to see what the problem is.

haraldg · 2017-01-11T08:30:06Z

acinclude.m4

@@ -9,7 +9,7 @@ AC_ARG_ENABLE(drivers,
 	[                  which is a comma-separated list of drivers.]
 	[                  Possible drivers are:]
 	[                    bayrad,CFontz,CFontzPacket,curses,CwLnx,ea65,]
-	[                    EyeboxOne,futaba,g15,glcd,glcdlib,glk,hd44780,i2500vfd,]
+	[                    dm140,EyeboxOne,futaba,g15,glcd,glcdlib,glk,hd44780,i2500vfd,]


please keep the alphabetic sorting

haraldg · 2017-01-11T08:32:10Z

dm140.c

+        if (p == NULL)
+        {
+                report(RPT_CRIT, "Failed to allocate memory for PrivateData\n");
+                return -1;


I think RPT_CRIT is a bit thouch. Other drivers use RPT_ERR

haraldg · 2017-01-11T08:36:34Z

acinclude.m4

@@ -128,6 +128,10 @@ dnl				else
 			DRIVERS="$DRIVERS debug${SO}"
 			actdrivers=["$actdrivers debug"]
 			;;
+		dm140)
+			DRIVERS="$DRIVERS dm140${SO}"
+			actdrivers=["$actdrivers dm140"]


Here you should ensure that all necessary dependencies are available.

haraldg · 2017-01-11T08:38:05Z

dm140.c

+  int i;
+
+  report(RPT_INFO, "%s called with values(x,y,c): %d, %d, %s", __FUNCTION__, x, y, buffer);
+


Probably you want debug() instead of report() here.

haraldg · 2017-01-11T08:39:15Z

dm140.c

+    VFDSetString(drvthis, y, 1, p->framebuf[i]);
+  }
+
+  // Don't know what to do


That's not very reassuring ...

haraldg · 2017-01-11T08:40:48Z

dm140.c

@@ -0,0 +1,241 @@
+/*
+ *  dm1400 vfd driver (c)2007 Henrik Larsson
+ */


Um, what's the license of this code? And I think it is lcdproc policy to only merge code with the actual authors consent...

haraldg · 2017-01-11T08:46:55Z

dm140.c

+	p->gFlags = 0;
+	p->gDisplayMode = VFD_MODE_NONE;
+
+	if ((p->framebuf = (char *) calloc(1, p->height)) == NULL)


AFAIKS this allocation doesn't match the declaration.

awiouy · 2017-01-11T09:12:15Z

My intention is only to have this merged upstream, for the benefit of everyone. I did not write the code and I would be unable to fix it. A quick search suggests that the author submitted it to lcdproc 0.5.3 (https://www.mythtv.org/wiki/Futaba). I do not know why it has not been merged, yet.

lcdproc devs could maybe take this up from here? Thank you in advance.

haraldg · 2017-01-11T09:21:37Z

My intention is only to have this merged upstream, for the benefit of everyone. I did not write the code and I would be unable to fix it. A quick search suggests that the author submitted it to lcdproc 0.5.3 (https://www.mythtv.org/wiki/Futaba). I do not know why it has not been merged, yet.

If you followed the link on this wiki page, it would have been obvious to you why it hasn't been merged yet: License isses. Like what I wrote an hour ago.

However a driver called futaba was merged recently. I don't know if this supports the same HW. You would need to chack that yourself.

lcdproc devs could maybe take this up from here?

Most certainly not.

GeoSimos · 2017-01-11T21:15:24Z

Hello,
The FUTABA driver merged before doesn't cover the "DM-140gink" device, it covers MDM166 which isn't compatible with DM-140 (or dm140).
There is also a patch for OpenELEC here: https://github.com/OpenELEC/OpenELEC.tv/blob/master/packages/sysutils/lcdproc/patches/lcdproc-0.5.6-dm140_henlar_v0.2.patch
Please you are kindly requested to add support for this popular LCD.

Thank you.

haraldg · 2017-01-11T23:19:20Z

Personally I'm not interested in working on drivers, for which I don't have the HW to test them on. And it seems nobody else is really active. If dm140 is similar enough to the HW supported by the existing futaba driver, your best bet probably is to ask the author of the futaba driver if he is interested in adding support to his driver. (Or find somebody else who cares enough to do the work.)

Since I feel somewhat pestered, I want to point out that drivers aren't added to lcdproc based on the popularity of the HW. Drivers are added if the HW is available easily enough (eg can be purchased new) AND somebody provides code of sufficient quality and proper licensing.

ElStupid · 2017-02-03T10:06:38Z

I would really love to see support for dm140 again too. It's a sad thing to see an empty display since a few months now.

sammy2142 · 2017-04-07T13:30:26Z

This driver does have some issues. A portion of the screen isn't used (the first 8 columns of pixels aren't used). It has 112 columns and 16 rows of pixels total.
When text is scrolling on the first row (it can fit two rows of text); the rightmost edge of the first row of text appears on leftmost edge of the 2nd row of text (only 1 pixel wide).
None of the special symbols appear to be working (Does kodi make use of these?? volume bar etc)
https://www.mythtv.org/w/images/3/30/Dm-140gink_front.jpg

JamesGKent · 2017-04-10T15:13:08Z

i can't comment on the pixel alignment issue, but from having tinkered with it i can say that the kodi client does try to use special symbols where they are available. so if possible they ought to be added into this driver, but as @haraldg has stated there is still the license issue with this driver.

JamesGKent · 2017-04-10T15:21:56Z

just to add, i looked at the patch used in openelec to incorporate this driver, and while legalese may not be my strong suit, the way i read it this driver uses files written by Advanced Micro Devices, Inc. with a GNU v2 header in them, as this is a derivative work, would that not mean this is GNU V2 aswell?

haraldg · 2017-04-11T09:55:23Z

the way i read it this driver uses files written by Advanced Micro Devices, Inc. with a GNU v2 header in them, as this is a derivative work, would that not mean this is GNU V2 aswell?

Unfortunately it's not that easy: If you mix something that is GPLv2 (only) with something else the result is not distributable at all, unless the "else" is a very permissive license like BSD. That's even true for mixing GPLv2 and GPLv3 when no special exemptions are granted. However I haven't looked into this specific case in detail, so maybe there is an easy solution - but then people have failed to point it out for many years. OTOH writing lcdproc drivers is fairly easy - everybody with basic C skills could rewrite the driver from scratch or from the GPLv2 file you found. Maybe that would be less work then investigating the situation. However it seems nobody cares enough to do it.

…

-- If you want to support my work: see http://friends.ccbib.org/harald/supporting/ or donate via peercoin to P98LRdhit3gZbHDBe7ta5jtXrMJUms4p7w or CLAM xASPBtezLNqj4cUe8MT5nZjthRSEjrRQXN

JamesGKent · 2017-04-11T10:53:48Z

fair enough, I wasn't sure if it was that simple, but then licenses aren't my strong suit.
and you are probably correct, at least no one who knows how cares enough to do it.
I'd give it a go except i have no display to test on, and i don't want to spend money on a display less capable that the ones i already use.

but as you point out there are issues with the driver as written, even the original author has admitted in various forum threads that there are some issues with it, along with some features not working at all (volume bar)
and given that this device has been seen with different vendor/product id's i'd suggest it ought to either make these configurable, add a list of known vid/pids that it tries, or even both. but i doubt that will happen unless someone with one of the displays is prepared to put in the work.

haraldg · 2017-04-11T11:25:28Z

On 11.04.2017 12:53, James wrote: even the original author has admitted in various forum threads that there are some issues with it

If people know how to contact the original author, then maybe he can be bothered to address the issues (legal and technical) in this PR.

Thecooler100 · 2021-11-03T17:55:18Z

This driver does have some issues. A portion of the screen isn't used (the first 8 columns of pixels aren't used). It has 112 columns and 16 rows of pixels total. When text is scrolling on the first row (it can fit two rows of text); the rightmost edge of the first row of text appears on leftmost edge of the 2nd row of text (only 1 pixel wide). None of the special symbols appear to be working (Does kodi make use of these?? volume bar etc) https://www.mythtv.org/w/images/3/30/Dm-140gink_front.jpg

I couldnt get my dm140 to work with kodi at all. I did set up xbmc-lcdproc properly but im getting nothing from the screen.

dm140: add driver

54d0ee7

haraldg requested changes Jan 11, 2017

View reviewed changes

CvH mentioned this pull request Jan 11, 2017

dm140 drivers #48

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dm140: add driver #47

dm140: add driver #47

awiouy commented Jan 11, 2017

awiouy commented Jan 11, 2017

haraldg left a comment

haraldg Jan 11, 2017

haraldg Jan 11, 2017

haraldg Jan 11, 2017

haraldg Jan 11, 2017

haraldg Jan 11, 2017

haraldg Jan 11, 2017

haraldg Jan 11, 2017

awiouy commented Jan 11, 2017

haraldg commented Jan 11, 2017

GeoSimos commented Jan 11, 2017 •

edited

haraldg commented Jan 11, 2017

ElStupid commented Feb 3, 2017

sammy2142 commented Apr 7, 2017

JamesGKent commented Apr 10, 2017

JamesGKent commented Apr 10, 2017

haraldg commented Apr 11, 2017 via email

JamesGKent commented Apr 11, 2017

haraldg commented Apr 11, 2017 via email

Thecooler100 commented Nov 3, 2021

		int i;

		report(RPT_INFO, "%s called with values(x,y,c): %d, %d, %s", __FUNCTION__, x, y, buffer);

dm140: add driver #47

Are you sure you want to change the base?

dm140: add driver #47

Conversation

awiouy commented Jan 11, 2017

awiouy commented Jan 11, 2017

haraldg left a comment

Choose a reason for hiding this comment

haraldg Jan 11, 2017

Choose a reason for hiding this comment

haraldg Jan 11, 2017

Choose a reason for hiding this comment

haraldg Jan 11, 2017

Choose a reason for hiding this comment

haraldg Jan 11, 2017

Choose a reason for hiding this comment

haraldg Jan 11, 2017

Choose a reason for hiding this comment

haraldg Jan 11, 2017

Choose a reason for hiding this comment

haraldg Jan 11, 2017

Choose a reason for hiding this comment

awiouy commented Jan 11, 2017

haraldg commented Jan 11, 2017

GeoSimos commented Jan 11, 2017 • edited

haraldg commented Jan 11, 2017

ElStupid commented Feb 3, 2017

sammy2142 commented Apr 7, 2017

JamesGKent commented Apr 10, 2017

JamesGKent commented Apr 10, 2017

haraldg commented Apr 11, 2017 via email

JamesGKent commented Apr 11, 2017

haraldg commented Apr 11, 2017 via email

Thecooler100 commented Nov 3, 2021

GeoSimos commented Jan 11, 2017 •

edited