Skip to content

Conversation

@mikee47
Copy link
Owner

@mikee47 mikee47 commented Jan 14, 2026

This PR fixes #80 so that when setting integer values they are clipped/clamped to the correct range rather than being truncated modulo-2. Values are now treated as signed 64-bit integers which are clamped as appropriate. The exception is for unsigned 64-bit storage type, which just uses uint64_t.

This affects:

  • Setting from from JSON (or other text sources)
  • Use of the generated set_xxx methods, which use the smallest appropriate data type for the value.
  • Setting integer array items

The test data has been updated to verify that this operates correctly.

Clamp parsed 32/64-bit value to 8/16-bit as required.
Use `simple-int` test value which exceeds int8_t range to verify
@mikee47 mikee47 changed the title Fix clipping of values set via string Fix clipping of integer values Jan 15, 2026
@pljakobs
Copy link
Contributor

These now accept 32-bit integer values (signed or unsigned as appropriate) which are clamped.

I would rather have the library throw an error if the number is out of range, but thinking about this, I've never tested that at all, what happens if I call updater.setValue(1024) on a property that only allows 0-64 for example? is that clamped to 0 or does the call return false?

@mikee47
Copy link
Owner Author

mikee47 commented Jan 16, 2026

what happens if I call updater.setValue(1024) on a property that only allows 0-64

It stores 64. Value is clamped/clipped to range. See #1 (comment)

@mikee47
Copy link
Owner Author

mikee47 commented Jan 16, 2026

I haven't really spent any time yet on error reporting #49 or change callbacks #48, both of which are mechanisms by which errors like these might be handled. At present, set_xxx methods all return void. If that were changed then application code complexity would increase considerably. We could always provide methods such as trySet_xxx which do return an error, then it's application choice.

Range errors can (and probably should) be managed more in the front end using standard HTML validation, using the ranges defined in the schema. If that is done, then at least database behaviour is predictable.

@pljakobs
Copy link
Contributor

pljakobs commented Jan 17, 2026

I've been thinking if it would be worthwhile to implement something like esp_idf uses where every function call returns ESP_ERROR but then, I think that's not really worth it - at least not for my application.

The challenge is, that the code does not "know" the min and max values for a given updater function, so it's really down to the developer to obey this range by knowing the schema.

It would potentially be good to have getters for those range borders, (like getValueMax() / getValueMin) but that sounds like a lot of extra code generated for no real use even though those functions would be really small.

How about a flag that is updated after every write to a store?

If I want to be cautious, I could write

   updater.setValue(10000);
   if(updater.lastUpdateClipped()) doSomething;

that would be a single value per store that could be updated with every set function and the developer would be free to check it to be sure.
Maybe instead of just a boolean, it could also be a very small enum of E_CLAMPED_UP and E_CLAMPED_DOWN and maybe we could throw in an E_REDUCE for a down cast?
This value would only be in RAM of course as it's only use is to check right after writing a value.

[edit]
thinking about it - the library never sees the original value, so it would have no way to know f it has been type cast down, so ignore the E_REDUCE value.

in a roundabout way that would be a way to derive the max and min for any property, just call it with the largest/smallest INT, check for E_CLAMPED and read back the value - that is the min/max for that field.

not straight forward, but workable.

@mikee47
Copy link
Owner Author

mikee47 commented Jan 17, 2026

I think this is wandering off-topic as this PR is just a bugfix - I'll merge this PR once CI completes.

thinking about it - the library never sees the original value, so it would have no way to know f it has been type cast down, so ignore the E_REDUCE value.

Last commit in this PR changes the parameter type to int64_t (or uint64_t as appropriate) for set_xx methods so mod-2 truncation (E_REDUCE) cannot occur. The clamping is done in-line so the compiler can optimise the code.

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.

8 and 16-bit values not converted from string correctly

3 participants