ddcutil .deb

[Dorian Vogel]

Hi all,
Thanks for the comments ! And sorry for the late reply.
I realize the implementation is far from optimal, and would be happy to
rework it. Unfortunately I'm quite busy with other things right now, and
for about two more weeks so I can not really swear anything until then.

Jonathan: keeping the feature "in development" for now sounds reasonable,
the fact that powerdevil's architecture does not allow ddc brightness
control for laptop might become a source of frustration for users.

Regards,
Dorian

Le 4 sept. 2017 2:07 PM, "Jonathan Riddell" <jr at jriddell.org> a écrit :

>
> Thanks for your comments Sanford.  The code isn't mine it's done by
> Dorian for KDE Plasma's Powerdevil which gets a beta release next week
> (I'm the release dude).
>
> Dorian please review ddcutil Sanford's comments below for changes that
> could be made.
>
> Sanford given the API/ABI stability should we be recommending distros
> package up libddcutil and use this code or would that cause more
> problems and we should mark it as in-development for this release?
>
> Jonathan
>
>
> ----- Forwarded message from Sanford Rockowitz <rockowitz at minsoft.com>
> -----
>
> Date: Fri, 1 Sep 2017 10:52:29 -0400
> From: Sanford Rockowitz <rockowitz at minsoft.com>
> To: Jonathan Riddell <jr at jriddell.org>
> Subject: Re: ddcutil .deb
>
> Jonathan,
>
> I've taken a look at your powerdevil code at
> https://cgit.kde.org/powerdevil.git/tree/daemon/backends/
> upower/ddcutilbrightness.cpp.
> It can be simplified to improve performance and reliability.
>
> 1)  The DDCA_Display_Identifier -> DDCA_Display_Ref lookup exists to
> allow for choosing a monitor by its characteristics, when you don't
> already have a list of monitors to select from, e.g. on the ddcutil
> command line.   Struct DDCA_Display_Info, returned (as an array) by
> ddca_display_info_list(), contains field dref, which is a valid
> DDCA_Display_Ref you can use.   See function
> display_selection_using_ddca_get_displays() in sample file
> demo_display_selection.c.
>
> The documentation could be clearer here.
>
> 2) There's no need to look up the VCP value for the Brightness field.
> Per the MCCS spec, it's always x10.
>
> 3) Rather than reading and parsing the capabilities string to
> determine if a monitor supports VCP feature code x10, it's both faster
> and more reliable just to try reading the value of code x10.
>
> Reading the capabilities string is a very expensive operation,
> entailing multiple I2C write/read exchanges, as a single read can
> return at most 32 bytes.  Every write/read exchange contains sleeps
> mandated by the DDC/CI protocol.  (In fact, ddcutil spends 90% of it's
> elapsed time sleeping.)    Since the I2C protocol is unreliable, a
> write/read exchange may have to be retried, and even worse a whole
> sequence of write/read exchanges may have be to restarted.   Most
> monitors are relatively clean, but some, such Dell P2412h monitors
> that I have tested, have so many I2C failures that read capabilities
> sometimes fails even after retries.
>
> Moreover, the capabilities string is not necessarily accurate. See,
> for example, the description of the HP LP2480zx monitor. on page
> http://www.ddcutil.com/monitor_notes/.  The string does not include
> VCP feature code x10, even though the monitor supports it.
>
> BTW, while monitors vary enormously in the VCP feature codes they
> support, I've yet to see a monitor that doesn't support feature x10.
>
> 4) You may want to control/capture messages issued from the library.
> See functions ddca_set_fout(), ddca_set_ferr(),
> ddca_set_output_level(), etc.
>
> 5) In certain very exceptional situations (i.e. things that should
> never happen, but hey, it's conceivable) the library may abort.
> Instead of aborting, the library can return to a location in the
> caller registered using function ddca_register_jmp_buf().  See
> function handle_library_abort() in sample file demo_global_settings.c.
> I have to admit that I'm not entirely comfortable with making a
> longjmp visible to the library client, but I'm trying to avoid forcing
> the library user to check for truly exceptional failures on each API
> call, while still avoiding library aborts that cause the client to
> fail.   Let me know if you find this design useful/usable.
>
> Regards,
> Sanford
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20170905/1afa8534/attachment-0001.html>