-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Observer Enhancement #7658
Comments
I agree with both.
|
I concur.
Agreed! And I cannot think of a use case (especially with regard to Widget binding) where one would want a notification every time a value got written to (including when it didn't change). Interestingly, this popped out at me when updating the Observer documentation. For the documentation to match the code, I had to write "... notification ... when ... value is updated" vs "when value is changed" (which seemed odd). |
It was a great catch indeed. I'd like to emphasis that no notification should be triggered when when setting the same value because of the philosophy of subject-observers: the observers are always in sync with the subject because when they subscribe a notification will be fired to get the current value (probably it should be documented too) and when the value changes a notification is fired as well. So there is no way that notifying with the same value makes any difference, unless the user messes up something, but it's out of scope now. 😄 |
Done. Will be submitted in a subsequent PR. I also included that the notification goes out ONLY when the value has changed (involving changes in about 4 places in the document). I will submit this with the code update. |
Introduce the problem
While I was re-writing and reviewing the documentation for Observer, I noticed a couple of logic gaps, which I am predicting will be needed in the future:
Currently FLAG and STATE combinations can be set or cleared by an Observer based on EQUALITY or INEQUALITY with a reference value. However, I am predicting this will be also needed for the following integer comparison operations with
<
,<=
,>=
,>
suggesting additionallv_obj_bind_...()
functions. Note that only the first 2 of those could be implemented plus a "not" flag to control the execution path during testing, so it would only take 3 bits to manage (2 more bits in theflag_and_cond_t
struct) to manage, no matter now it was organized. (No additional RAM cost.)Each Subject carries a
prev_value
field, but the only place it is actually used is in these query functions:lv_subject_get_previous_int()
lv_subject_get_previous_string()
lv_subject_get_previous_pointer()
lv_subject_get_previous_color()
While the latter is fine as a "minimal" implementation, there is an important UI design pattern that minimizes CPU overhead (extremely important in my opinion in some RTOS environments) that:
Only does an update on the Widgets ONCE immediately before each screen refresh, and
ONLY does so (for each Widget) when it will result in a visible change on the display, and this pattern especially avoids doing an update when the value displayed has not changed, since it incurs an
lv_obj_invalidate()
, which forces re-drawing, which is then a large-and-unnecessary source of CPU overhead that would result in no visible change on the display.This pattern begs for Observer-notification filtering such that (when desired) that only sends out a notification when the NEW (incoming) value (in the various
lv_subject_set_...()
andlv_subject_copy_string()
functions) is actually different from the previous value. All the logic (and data) to implement this capability this is already contained in the implementation except for the flag indicating when this filtering is desired.Proposal
That the above features be added. I would definitely use it in our Dashboards if it were available to set a "widget_needs_updating" flag for each Widget so affected. It would simplify a lot of logic I have currently implemented to accomplish this (embedded in each screen's
Update()
function -- I have a screen "class" that always has anUpdate()
function that gets called once, immediately before the display refresh).I was thinking these features could perhaps be planned for the next major release (low priority).
Thoughts?
The text was updated successfully, but these errors were encountered: