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

Restore Samba output to stdout; restore supported Samba protocol #7

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

evanchaney
Copy link

Address multiple problems which could prevent data from being retrieved
from an AirVisual Pro.

Writing output to stdout

Prior to this patch the smbget command was used with the -O flag
to retrieve data from an AirVisual Pro. Unfortunately, the -O flag
is not standardized across platforms.

For example, on Ubuntu and Fedora the -O flag manages options for the
TCP socket used to connect to the device.

On Debian and in the Samba reference implementation the -O flag means
“write output to standard out”.

Due to these differences, depending on which platform Homebridge is run
under the plugin would or would not work. Removing the -O flag simply
inverted the problem (platforms that were working would stop working
because the data was no longer available on stdout).

The --stdout flag does appear to be consistent across platforms. Due
to the SMB protocol issue, however, this issue was resolved by switching
to smbclient and issuing a command which uses stdout as the download
destination for the file.

Using a supported SMB protocol

It appears AirVisual Pro devices do not speak SMB protocol versions
2 or 3. Here in December 2024 it also appears Samba only supports SMB
versions 2 or 3 by default.

The Samba reference documentation (for smb.conf) mentions that many
Samba commands support an --option flag which can be used to set
configuration options on a per-connection basis. (I.e. without having
to edit the smb.conf file on the running system.)

The smbget command does appear to support this flag on Ubuntu and
Fedora but results in an unrecognized-option error on Debian.

To resolve this issue smbget is replaced with smbclient which does
appear to support the --option flag on all (examined) platforms. This
flag is then used to restore support for the NT1 version of the SMB
protocol which AirVisual Pro devices do appear to support. This solution
avoids requiring the user to make any changes to their Samba
configuration file.

In 66c6a51 the `-O` flag to `smbget` was removed. On some platforms
(e.g. Ubuntu Noble; Debian 12.6) this flag means “socket options”. But
on other platforms (and in the Samba reference implementation) this flag
means “write output to stdout”.

As a result of the flag’s removal the call to `smbget` was downloading
the received JSON to a file in the working directory rather than writing
it to `stdout`. As a result, the `refresh()` function did not receive
the data and air quality values were zero value.

It appears the `--stdout` flag may be universally supported across
platforms to mean “write output to stdout” which should restore air
quality data regardless of platform.
AirVisual Pro does not appear to support SMB protocol versions 2 or 3
which are the default versions supported by Samba circa December 2024.

Ubuntu Noble and Fedora 40 packages of Samba support an `--option`
flag which can be used with `smbget` to lower the supported protocol to
include NT1 without having to modify `smb.conf`.

Unfortunately, the Debian Samba package does not support this flag with
`smbget` and the flag does not appear in the official Samba man pages
for `smbget` on samba.org.

Fortunately, the `--option` flag _does_ appear to be supported on each
of Ubuntu Noble, Fedora 40, Debian 12.6 and in the official Samba
documentation for `smbclient`.

To resolve a silent failure to refresh data caused by a failed Samba
connection, rewrite the Samba command to use `smbclient` and lower the
minimum supported SMB protocol version to something AirVisual Pro can
speak.
@evanchaney
Copy link
Author

This likely addresses #6 and #4.

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.

1 participant