Calibration Data load/save interface

[steved]

I've been looking at loading and saving touch screen calibration data, and making the interface more generic.

It comes down to streams of bytes, so the same interface could be used with displays and other I/O devices as general configuration data (e.g. setting a display type or resolution)

I propose the following interface:

gfxError_t gfxSaveDeviceConfig(uint8_t deviceID, const uint8_t *data, uint16_t dataSize);
gfxError_t gfxReadDeviceConfig(uint8_t deviceID, uint8_t *data, uint16_t dataSize);

gfxError_t is a result/error code type whose values are to be determined (also on the TODO list); with codes for success, possibly 'not implemented', and various reasons for failure.

The caller can then take action (or ignore) as is appropriate - for example on failing to read touch screen calibration data, the calibration screen is displayed.

The underlying routines can then access whatever non-volatile store is available, and possibly even provide sensible defaults in some cases.

deviceID needs defining in some way, so that it can reliably reference input devices, displays and so on. Maybe a simple block of values for each device type; for example:

0x00..0x07 - touchscreens

0x08..0x0f - dials

0x80..0x83 - displays

Any comments?

[Joel Bodenmann]

Hello Steve

This sounds like a good idea and it's definitely suited for the future.

Both, Andrew and I, are very busy at the moment. As soon as we finished our current tasks, we'll take a look at this. I'm sure that Andrew has some more comments on his own as well :-P

~ Tectu

[steved]

Unless I get deflected, I shall probably code this up myself, next week.

Mostly want to be comfortable that the API is sensible and practical.

(I've already got another potential use, supporting a number of resolution-compatible displays. It could also be a way of supporting a number of different resolutions).

[inmarket]

This is something I have been thinking about for a while now. I have been thinking that there is potentially another layer of abstraction that can be applied here. That is as a general settings register much like the windows registry but without all the complications.

This background thinking was part of the recent addition of the gfile module. The register could be implemented as a file or pseudo file on any storage system that can be hooked into gfile.

Storage in raw flash blocks, in a ROM file, in a file on a FAT file system would all be possible.

The delay in making this happen is always time. Gfile is not complete enough yet and more api needs to be designed and tested.

We are currently working on adding FAT file system support to gfile. So perhaps we are getting close.

A contribution in the area of a general settings register appropriate for an embedded platform would be highly appreciated.

[inmarket]

One more comment. Specifically about the read call:

It is important to be able to determine the size of the buffer needed for the call. Their perhaps needs to be a call to return the size of the buffer needed for the read. As the write could save different sizes even for the one device type, the size of the data block written needs to be saved with the data itself.

The read call should probably therefore fail if the data block passed in is not large enough. It may be worthwhile for the read to return the size of the data actually returned with 0 indicating a failure.

[Joel Bodenmann]

Using the GFILE to be able to read and write these configs to any file sounds sane. However, I'd definitely keep the GFILE magic hidden behind a real uGFX configuration API. This could become part of the core system (gfx.c) in form of:

gfxConfigSave()
gfxConfigRead()
gfxConfigXxx()

About the buffer size: I'd recommend to make the first few bytes the information about the size of the actual content. One could then read out these few bytes, allocate the space and continue reading. This could either be implemented in form of gfxConfigGetSize() or simply doing it manually in gfxConfigRead().

~ Tectu

[steved]

I'd see GFILE as an option, rather than the only solution - for myself, initial implementation will write calibration and other information to non-volatile memory, without a file system in sight. This is probably typical of many embedded systems, which don't need the complexity/code space of a file system (even if someone's already solved the problem!). Although if its easy to point GFILE at an block of non-volatile memory.... (I use a small FRAM, communicating over I2C, since speed isn't at all important for small blocks of data. Each data block includes a size and a checksum). Keeping the general principle of ugfx, maybe we support GFILE and various direct memory-based arrangements using drivers.

The various comments seem to be building on the principle quite nicely:

1. We call it a 'registry' - so my data identifier becomes some sort of tag. We could leave the tag as a unit8_t, or extend it to uint16_t to give more flexibility. Split the tag's number space into two blocks; one for ugfx-related devices (first 64 tags, say), and one for user tags. It's then up to the registry driver how to assign a storage location; a simple non-volatile memory driver might have a table relating tags to start addresses, while a file system might create a file name based on the tag.

2. A separate 'get data size' call seems sensible for those situations where the data size is variable. Some (most?) uses will have a fixed or known maximum data size, and not need to use this call.

3. The read routine would certainly need to check that its not going to overflow the passed buffer. If we pass the buffer size by reference, it can then be filled in with the actual size of the data block on return, and either a 'success' return code or a 'buffer too small' error code.

(I don't think we want to get into passing data in chunks - that's surely a case for direct file system access. I would expect registry entries to be quite small.)

So the API becomes three entries:

gfxError_t gfxSaveRegistryData(uint16_t tag, const uint8_t *data, uint16_t dataSize);
gfxError_t gfxReadRegistryData(uint16_t tag, uint8_t *data, uint16_t *dataSize);
gfxError_t gfxReadRegistrySize(uint16_t tag, uint16_t *dataSize);

I've deliberately adopted the format for gfxReadRegistrySize() for consistency and helpfulness; it gives the opportunity to return error codes for 'invalid tag' and maybe 'no data stored', which is less ambiguous than simply returning zero for both these cases.

Edit: As an alternative to gfxReadRegistrySize(), if gfxReadRegistryData() accepts a null pointer for the data buffer, it could just return the data size.

[inmarket]

THis is turning into a module in its own right. Excellent!

What do you think of GREGISTRY or GCONFIG as the module name? I will use the GREGISTRY terminology below just for simplicity in the discussion.

As GFILE is light (it is not a full file-system just a shim layer) it makes sense to have GFILE able to access the raw devices as just a block of raw data/memory/device and then use GFILE to implement GREGISTRY. It gives a lot of flexibility to the implementation.

A uint16 tag identifier sounds great. I would suggest tags > GREGISTRY_MAX_USER_TAG be reserved for ugfx. That way the user can number their own blocks starting from 0 (nicer for the application).

Using a null pointer on the read call to get the size certainly works. More complicated from an API perspective but it seems to be the way most os's do that sort of thing. It is less obvious though.

I personally have a pet hate for "errno" style of error reporting (where the error is saved in a global variable). It is not thread-safe and it is non-obvious.

I personally also am not keen on passing by ref to get what is effectively a return value. I realise that conflicts with the above statement but that is just my particular design style. Of the two I am more opposed to the "errno" system.

I am wondering however for an embedded registry if it really matters why the read or write failed. It is good enough to know that it did and that you didn't get the setting you wanted. Debugging the system will have long ago told you of any systemic problem. For an embedded system fall-back in this type of situation really doesn't need to know why it failed, simply that it did. Using this embedded system simplification we can then can simplify the API...

So (as a suggestion):

bool_t gregistryWrite(uint16_t tag, const void *data, uint8_t size);
uint8_t gregistryRead(uint16_t tag, void *data, uint8_t size);
uint8_t gregistrySize(uint16_t tag);

Note that use of uint8_t. I am thinking that registry data should always be small. Is 255 bytes a reasonable limit or should this be a uint16_t with a GREGSITRY_MAX_DATA_SIZE #define?

What do you think?

[steved]

What do you think of GREGISTRY or GCONFIG as the module name?

GREGISTRY sounds the best choice to me

As GFILE is light (it is not a full file-system just a shim layer) it makes sense to have GFILE able to access the raw devices as just a block of raw data/memory/device and then use GFILE to implement GREGISTRY. It gives a lot of flexibility to the implementation.

Once the API is defined, it shouldn't matter whether GFILE is used or not; obviously helpful to some if there is a GFIL-based implementation. (My own applications may well want to manage NVR themselves, rather than letting GFILE do it).

A uint16 tag identifier sounds great. I would suggest tags > GREGISTRY_MAX_USER_TAG be reserved for ugfx. That way the user can number their own blocks starting from 0 (nicer for the application).

Sounds reasonable. To avoid losing registry data if GREGISTRY_MAX_USER_TAG is redefined during a code upgrade, I'd then assign uGfx tags growing down from GREGISTRY_MAX_TAG_VALUE (which will be 65535) - as with stack and heap.

Using a null pointer on the read call to get the size certainly works. More complicated from an API perspective but it seems to be the way most os's do that sort of thing. It is less obvious though.

It's a refinement which should be simple to implement; as I envisage the use of this module, most callers will already know the size or maximum size of the data. And if returning a 'buffer too small' error in response to a normal read, we could return the actual size of the data (discussion point - its a somewhat inconsistent return).

I personally have a pet hate for "errno" style of error reporting (where the error is saved in a global variable). It is not thread-safe and it is non-obvious.

I personally also am not keen on passing by ref to get what is effectively a return value. I realise that conflicts with the above statement but that is just my particular design style. Of the two I am more opposed to the "errno" system.

Me too.

I am wondering however for an embedded registry if it really matters why the read or write failed. It is good enough to know that it did and that you didn't get the setting you wanted. Debugging the system will have long ago told you of any systemic problem. For an embedded system fall-back in this type of situation really doesn't need to know why it failed, simply that it did.

At the most basic level, you are absolutely right. However, I have found from experience that logging the more detailed error code can help diagnose any subsequent problems - on low-volume systems debugging rarely covers all possible use cases (and sometimes, regardless of the amount of debugging/testing, coding errors can lurk undetected for years!). The logging need not be complex; a simple memory location which holds the last error encountered can be sufficient. And using a large number of different error codes can help home in on precisely where an error occurred. (Consider reading back data - a 'buffer too small' error will require different remedial action to 'storage device failure').

Note that use of uint8_t. I am thinking that registry data should always be small. Is 255 bytes a reasonable limit or should this be a uint16_t with a GREGSITRY_MAX_DATA_SIZE #define?

I considered the same question, and concluded that a reasonable limit is actually 256 bytes! Its a much more useful size than 255. So unless we have a special case of zero really meaning 256 (ugh) we need uint16_t with GREGISTRY_MAX_DATA_SIZE.

[inmarket]

GREGISTRY it is then.

Saving the error silently in a variable for debugger or logging use later sound acceptable to me in order to simplify the api.

Ordering from the top for system ones sounds fine. Or just use 0x7FFF for top user. The top bit then indicates a system or user tag.

I think you are right - 256 makes a good maximum data size so unit 16 it is.

I think that that is probably a good enough first api design. Have fun implementing it. I am looking forward to seeing it.

Just for your info - we have just completed work on fat file system support for GFILE. It is currently in a branch and should be merged with the master sometime tonight after final testing. The block interface is currently only written for Chibios but it would be simple to add replacement routines for other os.

[Joel Bodenmann]

I am sorry for my late reply. I have been quite busy these days.

Module name

As it makes sense to keep the module name as short as possible (see the following point), I'd strongly prefer to call the module GCONF instead.

API

I totally agree with the API listed by steved. However, the API style of uGFX is as the following: (); using camelCase. Following these rules, the API has to look like the following:

bool_t gconfWrite(uint16_t tag, const void *data, uint8_t size);
uint8_t gconfRead(uint16_t tag, void *data, uint8_t size);
uint8_t gconfSize(uint16_t tag);

I'm sorry but I am very pedantic when it comes to API and naming convention. I am already struggling with the older GDISP code that does not fit into the current naming style. However, this will be updated with the next major release.

Edit: I just realize that inmarket already adjusted the API naming accordingly in his second last post.

With everything else I agree. This looks like a solid base to start.

~ Tectu

[Joel Bodenmann]

After some more days of thinking I came to the conclusion that GREGISTRY might still be better than GCONF so we don't confuse new people with the configuration of the uGFX modules themself (gfxconf.h) etc.

~ Tectu

[steved]

After some more days of thinking I came to the conclusion that GREGISTRY might still be better than GCONF so we don't confuse new people with the configuration of the uGFX modules themself (gfxconf.h) etc.

~ Tectu

I agree - the term 'Registry' seemed to describe the function of the module well, so having a different name for the module and its entry points then seems perverse.