Re: CDL usage and improvements

[John Dallaway <john@dallaway.org.uk>]

Hi Jifl

Jonathan Larmour wrote:

> On 05/04/12 20:13, John Dallaway wrote:

>> Jonathan Larmour wrote:
> 
> [Wanting to stop using set_value]
> 
>>> Not really, no. Firstly, other targets use it. Secondly, the design intention
>>> for CDL is that targets should be defined by platform packages, albeit with
>>> "requires" rather than "set_value". As such this is much closer to the way
>>> things are intended to be. Yes it originated as a solution to a specific
>>> problem, but then so is ecos.db, which shouldn't exist at all. What we can do
>>> at the moment is make the transition to a future improved world easier, so that
>>> makes this approach better.
>> 
>> Jifl, there are just four other instances of the use of set_value in
>> ecos.db at present and in every case the command is actually unnecessary
>> as it sets the user_value of a CDL option to the same value as the
>> default_value.
> 
> Actually, you also need to equivalently consider the use "enable", which
> is pointlessly used by the snds, but usefully used by the adder / adderII.

I agree. So prior to your check-in we had 4 unnecessary instances of
"set_value", 3 instances of "enable" (of which 1 was unnecessary) and no
instances of "disable".

>> Discouraging the use of what is known to be a problematic
>> command seems entirely reasonable to me. I think you may be
>> underestimating how useful the "Restore Defaults" command is to regular
>> configtool users. Certainly I would not regard this command 'obscure' by
>> any means.
> 
> Okay, hands up any configtool users (not maintainers) out there who have
> used this command, AND it's done what they wanted.

No responses to date, but I think you would agree that this polling
technique is not the most objective. :-)

> Also, given that the config tool has a long-standing problem of invoking
> the inference engine incorrectly, I would consider "restore defaults" to
> be a rather risky operation - the graphical config tool's inference engine
> has the potential to do something different once things set via templates
> are unwound. It would certainly be extremely noisy. I would have thought
> no-one would use "restore defaults", but instead just create a new
> configuration. That's more straightforward, more familiar and more likely
> to work. "Restore defaults" is probably a misnomer, because it isn't the
> defaults you get when you create a new configuration (although if you are
> lucky it may end up that way after inference / conflict resolution).

In fact, "Restore Defaults" is much more useful when invoked at the CDL
package or CDL component level rather than across the entire eCos
configuration. Creating a new configuration is no substitute in these
scenarios.

>> Given that the design intention is to use platform packages to define
>> targets, I don't understand why you would regard using a "set_value"
>> command (located outside the HAL package hierarchy) as being closer to
>> how things are supposed to be.
> 
> Because the setting of options is associated with the target entry, which
> comes from the platform HAL package.
> 
>> I am suggesting that we use additional
>> platform packages to set configuration options appropriate for specific
>> targets. This seems absolutely aligned with the design intention which
>> allows a combination of HAL packages to describe the hardware.
> 
> I'm not saying it wouldn't work. But I'm against a pointless package
> cluttering up the target-side repository.

I don't think that using separate CDL packages to provide common and
board-specific parts of a platform HAL is so very different from the
partitioning of the HAL across multiple packages to accommodate
processor variants, or other functional blocks with some common
features. One of the great benefits of CDL is that it provides the
flexibility to accommodate abstractions that were not necessarily
envisaged at design-time. The capabilities are all there without the
need for the set_value/enable/disable hacks that deliver user_values
where default_values are required and were never intended to be used
outside the Red Hat test farm.

Jifl, it seems that both our views are quite firm. Since you have
already checked-in the new code and it is unlikely to impact too many
people, I am not inclined to argue this further. However, I would still
recommend that eCos developers avoid use of the "set_value", "enable"
and "disable" commands in ecos.db where possible.

John Dallaway
eCos maintainer
http://www.dallaway.org.uk/john