microsoft / jacdac

Device and service catalogs for Jacdac.
https://aka.ms/jacdac
Creative Commons Attribution 4.0 International
66 stars 25 forks source link

JD_SERVICE_NUMBER_CRC_ACK breaks service abstraction. #60

Closed tballmsft closed 3 years ago

tballmsft commented 3 years ago

JD_SERVICE_NUMBER_CRC_ACK is a command (sort of), but appears as a service number/index. Any reason this couldn't be a command in the control service?

pelikhan commented 3 years ago

@tballmsft i'm moving all these bugs to the JACDAC repo.

mmoskal commented 3 years ago

The actual CRC would need to go in the payload in that case, and because packets are word-aligned we would need to use additional 4 bytes for no good reason. Right now, the CRC goes in the command field. It's best thought of as a feature of the transport mechanism (as it's tied to the actual CRC value), so lower level than commands (in fact it's handled before any semantic analysis of commands).

mmoskal commented 3 years ago

Actually, think of it that way - CRC is a service that always runs at a specific service index, and handles its commands in a specific way (where the command code is the CRC).

Similarly for pipes.

tballmsft commented 3 years ago

OK. Why don't we put these transport services at the top of the service index range then?

tballmsft commented 3 years ago

I had previously been thinking that the control service (0) was the transport, but it's clear to me now that transport is a separate concept from the control service.

mmoskal commented 3 years ago

That would complicate indexing. Right now, service class = services[index] where services is payload of the announce packet. It is somewhat inconsistent with the control service I guess...

tballmsft commented 3 years ago

Sure, it makes sense that transport is "global".