Skip to content

linux: add MTU accessor #379

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.

Sign up for GitHub

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

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

linux: add MTU accessor #379

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions gatts_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,17 @@ func (om *objectManager) GetManagedObjects() (map[dbus.ObjectPath]map[string]map
type bluezChar struct {
props *prop.Properties
writeEvent func(client Connection, offset int, value []byte)

mtu int
}

func (c *bluezChar) ReadValue(options map[string]dbus.Variant) ([]byte, *dbus.Error) {
if mtu, ok := options["mtu"]; ok {
// Store the MTU, so it can be retrieved later.
mtuValue := mtu.Value().(uint16)
c.mtu = int(mtuValue)
}

// TODO: should we use the offset value? The BlueZ documentation doesn't
// clearly specify this. The go-bluetooth library doesn't, but I believe it
// should be respected.
Expand All @@ -61,6 +69,12 @@ func (c *bluezChar) ReadValue(options map[string]dbus.Variant) ([]byte, *dbus.Er
}

func (c *bluezChar) WriteValue(value []byte, options map[string]dbus.Variant) *dbus.Error {
if mtu, ok := options["mtu"]; ok {
// Store the MTU, so it can be retrieved later.
mtuValue := mtu.Value().(uint16)
c.mtu = int(mtuValue)
}

if c.writeEvent != nil {
// BlueZ doesn't seem to tell who did the write, so pass 0 always as the
// connection ID.
Expand Down Expand Up @@ -168,3 +182,8 @@ func (c *Characteristic) Write(p []byte) (n int, err error) {
}
return len(p), nil
}

// MTU returns the exchanged MTU value, or zero if not available.
func (c *Characteristic) MTU() int {
return c.char.mtu
}
Comment on lines +186 to +189
Copy link
Member

Choose a reason for hiding this comment

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

The MTU is, as far as I know, the property of a connection. A characteristic is local, and can be read/written from multiple connected devices. So it doesn't make much sense to expose it as part of Characteristic.
A better place would be bluetooth.Device (though I'm not sure BlueZ exposes the MTU like that).

Copy link
Author

Choose a reason for hiding this comment

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

It doesn't look like BlueZ exposes the MTU on the device: https://man.archlinux.org/man/extra/bluez-utils/org.bluez.Device.5.en

By looking around in the BlueZ source, the two ways that the MTU is exposed are:

  • Via the MTU property on the characteristic here
  • Via the mtu option passed to ReadValue and WriteValue here

By digging a little bit more it seems that the MTU property is the "Biggest possible MTU" and is updated whenever a new ATT channel is added so that it only grows. However, the mtu option seems to reflect the actual MTU of the connection.

Now, the reason I originally added this was to be able to maximize the size of writes, even when multiple devices are connected. It seems to me that doing writes with the MTU property will leave out devices that have a lower MTU. To me it seems the correct thing to do is keep the minimum MTU value and write using that. However, if any device re-negotiates its MTU we are stuck with the old one.

Does this make sense?

Copy link
Member

Choose a reason for hiding this comment

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

So, what you're saying is the BlueZ doesn't expose the minimum MTU size at all? (Since it only grows, there can be devices connected with a smaller MTU than is reported).

Copy link
Author

Choose a reason for hiding this comment

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

Yes, that's what I think, but I shall report back once I tested it.

Loading