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

Better API support for read only properties #96

Closed
jreidinger opened this issue Feb 15, 2022 · 6 comments
Closed

Better API support for read only properties #96

jreidinger opened this issue Feb 15, 2022 · 6 comments
Labels
d-installer relevant for https://github.com/yast/d-installer

Comments

@jreidinger
Copy link
Collaborator

Hi,
in newly added feature to support properties #86 there is missing better support for quite common read only properties.
Basically there are two main usage of read only properties that needs to send signal.

  1. just given property is changed. Now it needs to use dbus_attr_reader, attr_writer and dbus_watcher for that single property.
  2. bunch of read only properties are changed ( like read or refresh method ). It is now supported by dbus_attr_reader plus call PropertiesChanged which is good enough.

So if it would be possible to support better use case 1. something like dbus_ro_attr_accessor that combines that three calls.

@mvidner mvidner added the d-installer relevant for https://github.com/yast/d-installer label Mar 4, 2022
@mvidner
Copy link
Owner

mvidner commented Jun 14, 2022

The simple idea is:

  • read-write at the Ruby (implementation) side
  • read-only at the D-Bus (API) side

@mvidner
Copy link
Owner

mvidner commented Jun 14, 2022

In case we change the API because of this, see also #115 where we may need to change the dbus_watcher API.

mvidner added a commit that referenced this issue Jun 16, 2022
TODO:
e_c_s on dbus_reader (and some its callers transitively) is unused but
that may change if we consider
#96
"Better API support for read only properties"
mvidner added a commit that referenced this issue Jun 17, 2022
TODO:
e_c_s on dbus_reader (and some its callers transitively) is unused but
that may change if we consider
#96
"Better API support for read only properties"
mvidner added a commit that referenced this issue Jun 17, 2022
TODO:
e_c_s on dbus_reader (and some its callers transitively) is unused but
that may change if we consider
#96
"Better API support for read only properties"

Again remove the type argument from dbus_watcher and the
dbus_property_changed4 method, the optimization is not worth the API
complication.
mvidner added a commit that referenced this issue Jun 17, 2022
It will use dbus_watcher and result in dbus_properties_changed

TODO: test it
@mvidner
Copy link
Owner

mvidner commented Jun 17, 2022

Before:

dbus_attr_reader :foo, type
attr_writer      :foo
dbus_watcher     :foo

After, with the simple implementation change in #117:

attr_accessor :foo
dbus_reader   :foo, type  # must come second

@jreidinger what do you think?

mvidner added a commit that referenced this issue Jun 18, 2022
It will use dbus_watcher and result in dbus_properties_changed
@jreidinger
Copy link
Collaborator Author

For me new API looks reasonable. I expect similar it will looks for rw attr in dbus? so attr_accessor and then dbus_accessor?

@mvidner
Copy link
Owner

mvidner commented Jun 20, 2022

I expect similar it will looks for rw attr in dbus? so attr_accessor and then dbus_accessor?

Yes, and because it's a simpler case, it has a name: dbus_attr_accessor

A read-write property accessing an instance variable. A combination of attr_accessor and dbus_accessor.

mvidner added a commit that referenced this issue Jun 20, 2022
It will use dbus_watcher and result in dbus_properties_changed
@mvidner
Copy link
Owner

mvidner commented Jun 21, 2022

Solved in #117

@mvidner mvidner closed this as completed Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
d-installer relevant for https://github.com/yast/d-installer
Projects
None yet
Development

No branches or pull requests

2 participants