Decoder design questions before submitting a pull request

Larry Backstrom 8 hours ago

Hello,

I've extended one of the existing binary UDP decoders, which only covered a basic configuration of its protocol: other GPS data types weren't decoded, so positions stopped updating, and there were no DTCs, VIN or ACK replies. It now covers additional GPS and hardware data types, DTCs and the MIL, VIN, device status, more engine fields, and ACK replies for acked UDP. It also fixes a few existing issues, such as engine hours stored in seconds rather than milliseconds. Everything is covered by tests, and the build passes checkstyle.

Before I submit a pull request, I'd like to check a few design choices with you:

  1. Time scale. The device timestamps are GPS time, which runs 18 seconds ahead of UTC. I plan to subtract a fixed 18-second constant in the decoder so positions are stored in UTC. Is a hard-coded constant acceptable, or would you prefer a configuration key?

  2. New attributes. Some engine values have no standard Position key: check-engine lamp, engine running, DEF level, ambient temperature, brake and parking brake. Is there a naming convention you'd like me to follow?

  3. Existing attribute names. The current decoder stores fuel level under "fuel", which is liters elsewhere, when the device sends a percentage. It also stores ambient air temperature as "temp1", which conflicts with 1-Wire sensors. Is it acceptable to change these to "fuelLevel" and a separate ambient key, or should the old names stay for compatibility?

  4. Cell tower fallback. The device includes the cell tower (MCC, MNC, LAC, CID). When it loses GPS lock, it keeps sending the coordinates and time of the last fix, with a status bit showing GPS is unlocked. My plan is to mark those positions invalid, attach the cell tower as the network, and rely on geolocation.processInvalidPositions. Is that the intended approach?

  5. Pull request scope. The decoder ended up mostly rewritten, so I plan to submit it as a single pull request. Is that acceptable?

Thank you

Anton Tananaev 6 hours ago
  1. Probably not without some extremely good justification.

  2. Check other decoders.

  3. It is acceptable to fix if something is not mapped correctly.

  4. Yes.

  5. This will make it much harder for everyone. I strongly recommend breaking it down into smaller PRs.