hci: move the protocol stack into a subpackage - #478
Conversation
|
This actually was run on |
Prepares the HCI, ATT and L2CAP code to move into a subpackage. The move itself is a separate commit, so that this behaviour change can be tested on hardware with the file layout unchanged. ATT held a *Characteristic and called four of its methods. It now holds a charHandler, a set of functions that gatts_hci.go fills in, so the public API of Characteristic does not change. A struct of functions rather than an interface, because an interface call on this path costs about 5 kB of flash on TinyGo (measured on nano-rp2040 with examples/heartrate). addLocalAttribute and addLocalCharacteristic take a uint8 rather than a CharacteristicPermissions. ATT writes the value to the wire as a raw GATT properties byte and never reads the permission bits, so the GATT concept does not need to cross the boundary. The advertising report and the connection events were read as struct fields from gap_hci.go. They are now read through accessors, which is what a subpackage needs. Two hooks, setEventHandler and setLeEventHandler, let a controller consume an event or an LE subevent that this package does not know. That is the extension point for controller specific events, and it costs about 150 bytes. Also moves SetRandomAddress down into the HCI layer, widens the ACL CID to uint16 so that CIDs above 0xff are possible, and reads the reason out of a Disconnection Complete event. Flash on nano-rp2040 against dev: discover 41752 -> 41612, advertisement 56696 -> 57176, heartrate 60340 -> 60980.
The HCI, L2CAP and ATT code is now tinygo.org/x/bluetooth/hci. It has no build tags, so it builds and unit tests on the host, and it no longer carries the four board tags that kept it baremetal only. A board supplies an hci.Transport and calls hci.NewStack. Transport is the only hardware dependent part, so each of the four backends now asserts that its transport satisfies it. Transport also loses ReadByte, which nothing called. The exported names drop the prefixes that were needed in one flat package. ogfLECtrl becomes OGFLECtrl, attOpReadReq becomes OpReadReq, evtCmdComplete becomes EventCmdComplete and leMetaEventConnComplete becomes LEMetaConnComplete. ErrHCITimeout becomes hci.ErrTimeout. The root package aliases every error under its old name. The Read and Write methods on the packet types were byte serialisers with the shape of io.Reader and io.Writer but not the meaning. They are now unexported marshal and unmarshal. AttributeProtocolError moves to the ble package, and ATT uses it instead of a second copy of the same 17 codes. The ATT error response codes were duplicated between errors.go and att_hci.go. hci_cyw43439.go reached into the hci struct from another file in the same package. It now uses SendVendorCommandWithoutResponse, which is the same route a board outside this repository would take. go vet found unreachable code after the infinite loop in waitUntilResponse, which the build tags had hidden until now.
d7f7fd7 to
8b34319
Compare
|
Rebased and ready. |
There was a problem hiding this comment.
att_hci and the new att file were not git mv so it is difficult to see if the att definition really changed that much
There was a problem hiding this comment.
Same for hci and hci/hci, it was not git mv. Tracking and history is lost and comparison is difficult in this PR
|
@deadprogram would you be so kind to |
|
The files were moved with
The two low ones are just below the limit. The cause is the export renames in To see all four as renames, lower the limit:
I can make the two files cross the limit if you want it. If the export renames |
I guess we learn something everyday :D |
|
A correction to my last comment. I measured the split, and the 54 and 55 The measurements, with git as the judge:
So the split that I offered gives 49 and 49, and both files stay as a delete Two changes together do work. The Files tab compares the base with the head My suggestion is to leave the history as it is and read the moves with a lower That reports all four files as renames. But if you prefer the five pull request |
|
The hardware check is done, and both pull request descriptions now record it.
Two limits to note. The peripheral path is not yet checked on pico-w or esp32. That leaves only the rename question open on this one. |
2 participants
Moves the HCI protocol stack out of the root package and into
tinygo.org/x/bluetooth/hci, for the four reasons in the issue: separate thehardware from the protocol, make unit tests possible, leave room for a more
complete protocol, and let a board add vendor specific extensions.
This is the second of three stacked pull requests. It builds on #477
ble: move the core value types into a leaf package, and the tests that themove makes possible come in #476. Review #477 first.
What moved
hci.go,att_hci.goandl2cap_hci.goare nowhci/hci.go,hci/att.goand
hci/l2cap.go, with no build tags, so the protocol code builds andtests on the host. It carried
hci || ninafw || cyw43439 || espradiobefore,which is why it had no tests.
The four seams
hci.Transportand callshci.NewStack. It is the only hardware dependent part, and each of the fourbackends now asserts that its transport satisfies it.
SetEventHandlerandSetLEEventHandlerreceiveanything the package does not handle, and
SendVendorCommandsends thematching commands.
hci_cyw43439.goused to reach into thehcistruct fromanother file in the same package; it now goes through this route, which is
the same one a board outside this repository would take.
*Characteristic. It now holds anhci.CharacteristicHandler, a set of functions. A struct of functions ratherthan an interface, because an interface call on this path costs about 5 kB of
flash on TinyGo, measured on
nano-rp2040withexamples/heartrate.gap_hci.goreadh.advDataandh.connectDataas structfields. It now goes through accessors.
Two commits, in order
hci: decouple the protocol layers from the root package types, which keepsone package and the file layout unchanged, so the behaviour change is
reviewable on its own.
hci: move the protocol stack into a subpackage, plus the export renames.Size
-size=short, againstdev:hci hci_uartVerification
go vetandgo testover./ ./ble ./hci, both plain and with-race,make smoketest-linuxandmake smoketest-windows, plus a build of all fourHCI transports and the two
bledebugconfigurations.Run on hardware.
examples/discover, which is the central path, works on oneninafw board, one pico-w and one esp32, against
dev, against the move andagainst the tests.
examples/heartrate, which is the peripheral path, was alsochecked on a ninafw board with a host as the central: the service and
characteristic discovery, the MTU and the read permissions are the same in all
three. The peripheral path is not yet checked on pico-w or esp32.