More Highsierra work #7
Conversation
darwin/device.go
Outdated
| @@ -507,7 +507,8 @@ func (d *Device) HandleXpcEvent(event xpc.Dict, err error) { | |||
| case evtCharacteristicRead: | |||
| // Notification | |||
| c := d.conn(args) | |||
| if args.isNotification() != 0 { | |||
|
|
|||
| // if args.isNotification() != 0 { | |||
There was a problem hiding this comment.
Hi @jacobrosenthal this is great, thanks for working on this. I would suggest removing instead of commenting, please.
darwin/device.go
Outdated
| @@ -516,7 +517,7 @@ func (d *Device) HandleXpcEvent(event xpc.Dict, err error) { | |||
| sub.fn(args.data()) | |||
| } | |||
| break | |||
| } | |||
| // } | |||
| c.rspc <- m | |||
There was a problem hiding this comment.
Hi @jacobrosenthal once again, thanks for working on this. Now that you have removed the condition, this line of code is unreachable, and is why the build is failing, due to the go vet check.
There was a problem hiding this comment.
In other words, this line can, and should be removed, as it is no longer needed.
There was a problem hiding this comment.
@moogle19 done, but then that break means the go channel thing on the next line is never happening..
There was a problem hiding this comment.
The line c.rspc <- m is to send message m to the channel c.rspc if not handled by the preceding code. Since your change handled m, no need to send now. That is why go get says that line should be removed.
|
@jacobrosenthal pretty please with sugar on top! |
jacobrosenthal
force-pushed
the
highsierra2-upstream
branch
2 times, most recently
from
c7b611c
to
2e4f05c
Compare
jacobrosenthal
force-pushed
the
highsierra2-upstream
branch
from
2e4f05c
to
c966721
Compare
|
Rebased to pull that commented code out. I would test this though, Im not a go guy and I barely understand that code flow. Also fixed a few more xpcids from reports on the noble side. |
|
@moogle19 seems like this is ready to merge, no? |
|
So I've found via noble the args.isNotification wasn't a goble implementation anomaly, but was actually removed from the xpcids. So I'm now confident in that removal. Weve shipped these xpcids in noble 1.8.0 as of today. |
|
@jacobrosenthal Thanks a lot! |
ping #3