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

Allow different types of write #55

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

mjg59
Copy link

@mjg59 mjg59 commented Jan 27, 2020

Some devices are sensitive about the type of write performed. Add support
for choosing whether or not a response is requested.

Some devices are sensitive about the type of write performed. Add support
for choosing whether or not a response is requested.
try:
self._object.WriteValue(
bytes,
{'offset': dbus.UInt16(offset, variant_level=1)},
{'offset': dbus.UInt16(offset, variant_level=1),
'type': write_type},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your PR! It's important that we don't break existing projects using this library. Since this parameter was previously not specified, what is the default value of type if omitted?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I read the PR, the PR proposes the write_value() function to receive a new optional parameter with_response=True and thus existing code not providing that explicitly will get the default of True; in turn that will set write_type to "request" and that is then given to type if omitted.
So, that is the long story; and in short: existing code would with this PR default to 'type': "request" (as they will default to the with_response=True parameter.

write_type = "request"
else:
write_type = "command"

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

write_type = 'request' if with_response else 'command'

might be more appropriate here, or if more explicit code is preferred:

if with_response:
    write_type = 'request'
else:
    write_type = 'command'

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.

3 participants