Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configuration Specifications #104

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Configuration Specifications #104

wants to merge 10 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Aug 21, 2018

Dear LCDcommunity,

in the course of my Master Thesis I will write a specification for your configuration with the help of the tool libelektra. I already implemented most of LCDd but still have alot of questions concerning some configuration settings.

Specifications for your 3 clients will follow in the next week(s).

Please take a look at the added Questions.md. Once everything is clear I will move out the specification from the created folder and delete the Questions.md file.

@haraldg
Copy link
Collaborator

haraldg commented Aug 21, 2018 via email

@ghost
Copy link
Author

ghost commented Aug 22, 2018

Hi Harald,

thank you for your quick answer!

You might want to coordinate your efforts with the other people on your
team (I don't really know who they are) or maybe even pester them into
shareing some preliminary PRs like you did. :)

I definitely will, but the colleague is currently on vacation ;)

I'm not sure I understand this one. The prefix clearly indicates an
hexadecimal number, I do assume all hexadecimal digits are valid. Why
do you have doubts?

I do not come from the C field of programming and do not have anything to do with hex numbers. I just wanted to ask to be sure.

But also note that
many of the drivers are quite obsolete and unmaintained and it might
not be worth putting lot's of effort into them.

Do you have a list of these drivers? If something is particularly complicated in an obsolete driver I will simply ignore a validation. Otherwise I try to support it :)

Also I don't know how to answer many of your questions about valid values.

You are right, I added some validation to at least prohibit negative values / false types.

An other problem with validation is, that many enumeration values have synonyms.

Can you provide an example of such a (not well documented) synonym usage?

There are multiple configuration items, each with the key "key" and a
string value of the format ("%d,%s", linux_key_code, lcdproc_key_name).

Ok, so the value is clear that it always has this format. But the key is still unclear to me.
In elektra an array looks like this: somekey/#0, somekey/#1, somekey/#2 etc.
How does it look in your specific case. Can you give me an example?

Um, I guess you would need to look up the USB spec for valid values?

I could probably do that but I think the maintenance effort to maybe synchronize with new USB standards outweighs the possible benefits of users being to sloppy when typing in a wrong ID.


I removed some questions now. Can you look over it for a second time and see if there is anything
which maybe affects you(r code)?

Otherwise I will use the git blame and pester your fellow contributors ;)

Thank you for your help!

Best regards,
Mike

@JamesGKent
Copy link

JamesGKent commented Aug 22, 2018

i can't add much here, but with regard to USB VID and PID, trying to somehow validate against a list of existing devices would be a fruitless exercise, however the USB standard allocates a 16 bit field to each of the VID and PID, as such the simplest and arguably most useful validation would be to ensure that it is a valid hexadecimal number (but maybe without the 0x prefix? not sure what the drivers look for, but would be clearer if the prefix is always present) between 0x0 and 0xFFFF

@shenghaoyang
Copy link
Contributor

Hi,

With regards to some of your questions on serialPOS, take a look at the documentation file for it in the doc directory. Also note that cell size is not the same as display size. 16x2 isn't a real sensible default for cell size - take a peek at the sample configuration file for LCDd, and refer to the serialPOS section.

I believe you'll also find some information regarding Custom_characters there.

As for your limits, some protocols themselves impose a maximum size on the display (at least for serialPOS - see the CD5220 and AEDEX protocols), and other attributes. If you really want to go full ham you can do some research regarding that.

Good luck!

@haraldg
Copy link
Collaborator

haraldg commented Aug 22, 2018 via email

@ghost
Copy link
Author

ghost commented Aug 23, 2018

16x2 isn't a real sensible default for cell size - take a peek at the sample configuration file for LCDd, and refer to the serialPOS section.

Thank you, I updated the specification and also added more text to Custom characters.

I also updated some more specifications since I read through some of your doc files.

The rest looks like to be pestering people.

Can someone answer the last 2 questions from the "Server" part? I think that this is also something on which everybody works.

Thanks for all of your help!

@ghost
Copy link
Author

ghost commented Sep 1, 2018

Alright, I have some further questions and will now address it to the corresponding people:

@marschap In glcdlib you provided a setting CharEncoding: Can you enumerate all the standards for the the specifications?

@marschap In hd44780 you made the setting "Speed" for the bitrate of the serial port. Is there any allowed range or enumerations like in all other settings for speed?

@marschap In IOWorrior: Whats the default for ExtendedMode?

@marschap In IOWorrior: Is there any validation possible for SerialNumber? Eg. 8 hex/decimal numbers

@marschap In lirc: what does prog mean? Any validation possible?

@marschap In xosd: Is there any range limit in Offset?

@marschap In xosd: Is there any validation possible for Font or any random string allowed?

@marschap In Server at Hello&Goodbye: Is there any validation possible, eg. a maximum number of allowed characters?

@haraldg I cannot find the user @mmdolze for glcd/rawserial questions. Are these drivers still active?

@haraldg In Server/FrameInterval: Are there any allowed enumerations or is just a positive number is required?

Thanks for your help.

@haraldg
Copy link
Collaborator

haraldg commented Sep 2, 2018 via email

@ghost
Copy link
Author

ghost commented Sep 4, 2018

@marschap lcdproc client: OnTime/OffTime ... Can you provide any description for these values? Any validation possible?

@marschap lcdproc client: Reportlevel: What is the range? 1-5?

@marschap lcdproc client: Delay: Just a positive number required? 0 inlcuded? No Description is given.

@marschap lcdproc client: ShowInvisible: No description is given

@haraldg is marschap still active since he does not answer any of my questions?

@haraldg
Copy link
Collaborator

haraldg commented Sep 4, 2018 via email

@kodebach
Copy link

kodebach commented Sep 5, 2018

Hi,

You might want to coordinate your efforts with the other people on your
team (I don't really know who they are) or maybe even pester them into
shareing some preliminary PRs like you did. :)

I definitely will, but the colleague is currently on vacation ;)

I am that guy :)

Until now I was focused on implementing a version of lcdproc using the old low-level Elektra-API, which doesn't use a specification and will only be used for comparisons in my thesis.

I will provide some code as soon as possible. For now I too have some questions:

  1. Should the new version of lcdproc be able to use the existing configuration format, or would it be enough, if there was some kind of automated script for updating to the new libelektra format?

  2. Is there some way of testing all the drivers, except for manually getting the hardware and checking for problems?

  3. Are the following configuration values the only ones that can occur repeatedly, i.e. are used like arrays?

    • Driver, Hello and GoodBye in the [Server] section of LCDd.conf
    • key in the [linux_input] section of LCDd.conf
    • Backlight in the [hd44780] section of LCDd.conf
    • Entry in any menu section in lcdexec.conf
    • Parameter in any command section in lcdexec.conf
    • String in any parameter section with Type=Ring in lcdexec.conf

    (found through grep -re 'config_get_.*([^,]*, [^,]*, [^0]' .)

Edit
4) One more question: @haraldg Is there any special reason, why you added the following lines to .gitignore, and thereby ignored these folders?

lcdproc
lcdexec
lcdvc

@haraldg
Copy link
Collaborator

haraldg commented Sep 6, 2018 via email

@shenghaoyang
Copy link
Contributor

IIRC some of the serial drivers that just blit data to the display can be tested if you hook them up to a pty slave and record the output for validation.

It's even better if you build a simulator, but I think that's really a hassle.

@kodebach
Copy link

kodebach commented Sep 8, 2018

How is the validation going to work? Currently we have a bit of validation coded in C directly into the drivers. Can we get rid of this with the specification?

That depends on the validation. Many things like range checks, regex checking strings, etc. can easily be checked by elektra. More advanced things like checking whether the .so file for a driver can be found or not will still have to be manually checked in code.

Is the elektra validation only performed when changing the configuration via elektra means or whenever loading the configuration?

I did some tests, and as far as I can tell at least some checks (e.g. from the type plugin) are only executed when kdb set is called. This would also mean that changing a mounted configuration file and then calling kdb get could result in unvalidated results or errors.
@markus2330 please clarify

lcdproc is used on embedded devices too where code size (and memory foot print) are an issue.

What kind of embedded devices? The lcdproc documentation (which seems kind of outdated but still) mentions that an x86 system with a POSIX compliant OS is required. AFAIK elektra this should suffice for elektra to run. Maybe @markus2330 can also expand on this.

In a nice world we would have elektra validation on updating the configuration and optional elektra validation at runtime for non-embedded systems and be able to strip in code validation completely.

Yes, I agree. @markus2330 would this be possible somehow?

Also we should probably agree on some order in which the various parts of the code are worked on

I will probably start with the the lcdvc and lcdproc clients. After that we will see...

What do you mean with configuration format?

I was mostly asking whether the new version should be able to directly work with an existing LCDd.conf for example. The alternative would be to provide a separate script, which users would need to run first to convert old configuration files. In any case my goal is that config files won't need to be updated manually.

I don't think anybody can answer this question reliably. Your check might miss some cases where statements are split over several lines, but otherwise looks good.

I will try some more advanced checks, but thanks. The reason for my asking is that it we need to write the specification differently, if a key can occur repeatedly (it represents an array).

@markus2330
Copy link

What is the objective of that comparison?

Using the low-level API should be much easier to do but does not add type safety. The objective is to have some idea how expensive (in work hours) the different ways of elektrifying applications are.

How is the validation going to work? Currently we have a bit of validation coded in C directly into the drivers. Can we get rid of this with the specification?

Yes, the goal is to get rid of all the C code doing checks or transformations.

Is the elektra validation only performed when changing the configuration via elektra means or whenever loading the configuration?

Within the low-level API validation only takes place when changing the configuration. This is safe as long Elektra is not bypassed (and config files do not vanish). The higher-level API then also add run-time and type-safety guarantees (not yet fully implemented). The goal is that the getter functions always return valid configuration (by using in-memory specs and exit() on memerrors).

In a nice world we would have elektra validation on updating the configuration and optional elektra validation at runtime for non-embedded systems and be able to strip in code validation completely.

The validation happens within plugins. So when someone does not install the plugins and we ignore some warnings, everything should be fine. We cannot provide any run-time guarantees then, tough. Maybe @kodebach can compare the code sizes so that we know if this reduction is noticeable at all? @haraldg Can you maybe give us some "acceptable" sizes for validation code? In the best case LCDproc without config code + Elektra + essential plugins is about the same size as LCDproc now. But that remains to be seen.

@Piankero is thinking about having default value calculations in Python. They certainly would need to be optional. @haraldg any thoughts about which interpreter (if any at all) would be acceptable to calculate default values? Should we open a new issue for this?

@haraldg
Copy link
Collaborator

haraldg commented Sep 8, 2018 via email

@haraldg
Copy link
Collaborator

haraldg commented Sep 8, 2018 via email

@kodebach
Copy link

kodebach commented Sep 9, 2018

Maybe something like:

  1. lcdproc client
  2. autoconf integration
  3. LCDd + curses driver
  4. ...

Works for me. I only mentioned lcdvc because it is so small that it basically wouldn't make any difference in effort.

@Piankero is thinking about having default value calculations in Python. They certainly would need to be optional. @haraldg any thoughts about which interpreter (if any at all) would be acceptable to calculate default values? Should we open a new issue for this?

Couldn't we use the mathcheck plugin or something similar? I think any real scripting language would be overkill for default value calculation. Anything that uses more then some basic math expressions and ifs probably shouldn't be used as a default value anyway. A default value should always be available, the more complicated the calculation gets, the easier it gets to overlook some case where no default value can be calculated.

@ghost
Copy link
Author

ghost commented Sep 9, 2018

Couldn't we use the mathcheck plugin or something similar? I think any real scripting language would be overkill for default value calculation. Anything that uses more then some basic math expressions and ifs probably shouldn't be used as a default value anyway. A default value should always be available, the more complicated the calculation gets, the easier it gets to overlook some case where no default value can be calculated.

Well, there are so many use cases which a simple math + if plugin are not sufficient enough. After seeing through some configurations it has to be a scripting language. Of course if the calculation is too complicated it should get into the code itself and maybe not the specification but this is up to the spec + code maintainer. In this case its defenitely worth to be in the specification.

@haraldg it is neither build/run/install time. In my proposed idea the python code just gets executed, once a user changes a setting. Take the Size setting as an example in your configuration. The python code would be executed only if someone would change the Model setting. It does not affect the run time at all.

@kodebach
Copy link

kodebach commented Sep 9, 2018

Well, there are so many use cases which a simple math + if plugin are not sufficient enough.
Could you give some examples?

As far as i can see the Size setting is just:

[/CwLnx/Size]
default = (../Model == 12232) ? 20x4 : ( (../Model == 12832) ? 21x4 : 16x2)

So possible a kind of switch expression would be useful too, but you would definitely not need a full scripting language with functions, loops, variables etc.

@ghost
Copy link
Author

ghost commented Sep 9, 2018

So possible a kind of switch expression would be useful too, but you would definitely not need a full scripting language with functions, loops, variables etc.

Your above example with the conditionals plugin did not work for me when I tried it a week ago.

For LCDproc it is not needed to have a full scripting language because default value calculation is just a simple switch expression, thats right. But as far as it goes for other configurations it will not suffice very quickly.

Imagine cases such as:

  • Size calculations from multiple other settings where users put in i.e. "4GB, 128MB" etc: There is no possibility to just extract a part of a string and make a calculation with it
  • Prohibit values (by throwing an Err in the calculation process) to hinder the user from setting a value if another value is already present: Cassandra uses this for example (you may either have listen_address or listen_interface set). This is no "default value" calculation per se but this potentially bigger plugin has more advantages.
  • Get the username/current IP/timezone/cpu threads/RAM/free space/ etc. from the system as default value
  • Check if a desired port is open as default, if not try desired port+1/2/3/4/5 etc.
  • much much much more

So before making small plugins which cover some specific needs for certain configurations I would suggest that we make a much more powerful plugin. Of course every user should keep code there as simple as possible or if possible do not use it at all but it would greatly relieve from the burden to make all these checks and calculations in the respective application in which they honestly should not belong to.

@ghost
Copy link
Author

ghost commented Sep 9, 2018

Nonetheless the default value calculation plugin idea should be discussed in our repo :)

Here we are interested if this approach when trying to set a value could trigger the change of another value (if not already present) via python is acceptable for the LCDproc community. This code would just execute once a user changes via kdb set!

@kodebach
Copy link

kodebach commented Sep 9, 2018

Nonetheless the default value calculation plugin idea should be discussed in our repo :)

I agree, but like you said yourself for LCDproc (and probably most other cases) no scripting language is needed. I propose we create a plugin that uses simple expressions and ifs that will be used for LCDproc. (the additional effort would be minimal because code already exists in mathcheck and conditionals) This way we do not need to decide which scripting language is acceptable under which circumstances.

Whether or not a plugin using python would be useful can then be discussed in the libelektra repo.

@haraldg
Copy link
Collaborator

haraldg commented Sep 9, 2018

I think I now understand what problem you are trying to solve with default value calculation. I haven't made up my mind yet, if I think it is worth solving on the elektra side.

If the script only runs at configuration updates, then which data is changed and how is it made persistent? I think I don't understand what solution you are proposing yet.

Anyway, you can't expect any scripting language other then /bin/sh to be available on the target where lcdproc is running.

@markus2330
Copy link

markus2330 commented Sep 9, 2018

haraldg wrote:

Of course. The idea is, that on embedded systems the configuration couldn't be changed (without going through some elektra tool, ie a webinterface) - so the stored configuration should be guaranteed to be valid.

If there is any way to change configuration via any Elektra tool, the validation plugins need to be present on the device to provide safety. (Btw. for me this is run-time, even if it happens in another process then LCDproc.)

I doubt it. I don't have the numbers ready ATM, but probably Elektra +
essential plugins is already larger then LCDproc now.

Yes, easily possible. I thought there are more drivers or they are more complicated. We won't have much success with trying to reduce the size of a 138k binary. (Elektra's core alone is 64k. Both sizes from Debian Stretch binaries on amd64.)

Build time, install time or run time?

We had questions for both build time and run time. The build-time dep can easily avoided (rewriting a small python script in C) but avoiding the run-time dep requires us to add more default value calculation plugins. This is a major thing but might be needed anyway. We need to discuss this internally first (ElektraInitiative/libelektra#2231) to not bother you too much.

@dod38fr
Copy link
Contributor

dod38fr commented Sep 10, 2018

<digression>

As far as i can see the Size setting is just:

[/CwLnx/Size]
default = (../Model == 12232) ? 20x4 : ( (../Model == 12832) ? 21x4 : 16x2)

Duh, that's a point I missed in LCDd model for Config::Model. I'll add that soon...
</digression>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants