Subject: Re: bug#43252: 27.1; DBus properties lack type hints or overrides

From: Hugh Daschbach <[email-masked]>
Date: Thu, 17 Sep 2020 11:42:09 -0700

Michael Albinus writes:

> Hugh Daschbach <[email-masked]> writes:
>
> Hi Hugh,
>
>>> this succeeds, we could implement a counterpart to the 
>>> dbus-monitor
>>> program in Elisp. And you would be able to access this 
>>> information
>>
>> Excellent.  I can now parse the output of dbus-monitor.  But 
>> capturing
>
> Implementation is more complex than expected. Due to its nature,
> org.freedesktop.DBus.Monitoring.BecomeMonitor requires another
> (parallel) connection to the bus. This is not foreseen yet in 
> dbusbind.c;
> will see how it could fly.
>
> What I could provide just now is an implementation which runs in
> *another* Emacs instance. This could be used for monitoring 
> only,
> because it is another connection to the bus per definition. Are 
> you
> interested to get such a partial implementation?

I'm interested in whatever you want to implement.  I see signature
verification useful for testing rather than an exposed feature.

>From what I can see from looking at dbus-monitor output the 
correct
property types are being exposed now.  It seems to be working.

So whatever we approach we take, the benefit is early warning of 
future
regressions.  You are a better judge of benefit of additional 
effort
than I.

A second Emacs instance seems to offer the same asynchronous 
output
gathering issues that dbus-monitor poses.  It does eliminates the 
ad-hoc
parser.

If you have a longer term goal, I'd suggest pursuing that rather 
than
something partial that you'll want to replace later.

But I have no objection to a parallel instance to gather request
signatures.

>>  Do you have
>> an issue with rolling that up in a macro? 
>
> No objection. But comments :-)
>
>> (defmacro dbus-test05-test-property (name value expected)
>>  `(let ((byte-array ,name))
>
> I wouldn't call the variable "byte-array"; it could be anything 
> during
> test. Call it "property" or alike.

Fixed

>>    (should
>>     (equal
>>      (dbus-register-property
>>       :session dbus--test-service dbus--test-path
>>       dbus--test-interface ,name :read
>
> I would use access type :readwrite. We want also to test 
> dbus-set-property.

Yes, I've added a set property test.  I'll move access to a 
parameter so
I can do both positive and negative testing; confirm that :read 
prevents
writes.

Which raises the question, should dbus-set-property function call 
fail
for a local property that isn't :readwrite, or should that only be
prevented by incoming messages?  Do we require that
dbus-register-property be used to update a :read access property.

>>       ,value)
>>      `((:property :session ,,dbus--test-interface ,,name)
>>        (,dbus--test-service ,,dbus--test-path))))
>
> What are the double commas good for? Typos?

I had nested quasi-quoted expressions.  I'm working to avoid that. 
So
that was a bug.

>>    (should
>>     (equal
>>      (dbus-get-property
>>       :session dbus--test-service dbus--test-path
>>       dbus--test-interface ,name)
>>      ,expected))
>>
>>    ;; a test for `dbus-get-property' shall be added.
>
> That's my typo - dbus-set-property is meant. And yes, it shall 
> also be
> here. So you might need macro arguments value1 expected1 value2 
> expected2.

I assumed as much.  I just carried the comment around blindly. 
I've
changed what I sent you to accept a list of pairs of values and 
expected
return sexps.  I use the first pair on the list for
dbus-register-property, verify retrieval, then use 
dbus-set-property to
update and verify the property from the remaining pairs.

I need more testing and a cleanup pass.  I'll pass along a better 
version when
I think it's ready for review.

I've started a few "should fail" tests.

Cheers,
Hugh