Draft for Discussion: mlawlerau

[mlawlerau]

Hi Mickey, my first GitHub PR so i might be missing or confused about somethings so help me out if I've got something wrong. Here is a draft PR of all my current changes. I've documented them below. Not expecting to complete this PR, just wanted you to see what I have been doing. Happy to discuss why i changed things and perhaps you might suggest better ways of doing things. I am new at objective-c and swift, coming from a java/groovy background.

LTODB2Adapter

• Made enums Swift friendly

LTOBD2Command

• Added 'LTResponseDataType' enum
• Extended concepts of Command to support accessing the response value in a typed manner independently of a formatted string.
  • i.e. for a temperature, you can access the reponse as an integer or a double value that can be used in an application directly
  • this requires inserting a new conceptual layer 'decodeResponse' which can turn the unpacked array of bytes into a typed object, but not convert to string or apply formatting/units
  • formatted string variant of response value still available - formattedResponse() simply calls decodeResponse() to do the decoding first, before it applies formatting and units.
  • units and format strings generalised as configurable properties for abstract/concrete Command classes, enabling sensible defaults and overrides where necessary.
• Introduced new method 'decodeResponse'
• Renamed 'cookedResponse' to 'responsePayload'
• Renamed 'didCookResponse' to 'didUnpackResponsePayload'

LTOBD2PID

• Added 'LTOBD2PIDInteger' and 'LTOBD2PIDDouble' abstract subclassses for PIDs whose response payload decodes to single Integer and Double objects.
• Enhanced concepts of a PID command to support
• Updated various PID commands to extend new abstract subclassses.
• More PID commands for query supported PID commands in additional hex ranges
• Odometer command (untested, not supported by my car)

LTVIN

• Added support for Model Year Range Discrimination (alas doesnt work for Porsche 997.2, plan to fix with pragmatic workaround)

LTBTLESerialTransporter

• Added 'adapter' property to expose CBPeripheral
• Temporary workaround to ignore Bluetooth BatteryMonitor
  • My car's Bluetooth Battery Monitor advertises with the same characteristcs as my ELM327 adapter.
  • Plan to extend functionality to support a known set of user-configurable identifiers to be excluded from connection

Localizable.strings

• Minor typos
• Add strings for Model Year decoding

[mickeyl]

First off, sorry for the delay in answering. I'm super busy with a new project (more on that later, but it has to do with car diagnostics).

Thanks a lot for your pull request. I will start commenting on the individual parts of the pull request in detail right after finishing this message. In general, what we should aim for is more fine granular commits, so that we have a chance to retrace the history.

Please see the inline comments. Looking forward to merging this contribution!

[mlawlerau]

No worries, we are all busy - this is not my full-time job either.

Completely agree that more fine grained commits is preferred - will try to do this.

Had a quick scan of your comments - look good, and will respond in detail.

Happy to be advised on style and C++ / Objective C / iOS style - I am coming from 20 years in enterprise java land.

I’m passionate about getting it right and making it understandable by others, so happy to iterate on contributions.

On 26 Mar 2021, at 08:53, Dr. Mickey Lauer @.***> wrote:

First off, sorry for the delay in answering. I'm super busy with a new project (more on that later, but it has to do with car diagnostics).

Thanks a lot for your pull request. I will start commenting on the individual parts of the pull request in detail right after finishing this message. In general, what we should aim for is more fine granular commits, so that we have a chance to retrace the history.

Please see the inline comments. Looking forward to merging this contribution!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/mickeyl/LTSupportAutomotive/pull/36#issuecomment-808048067, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKN7RYPAFLIEUOXIBSQYO23TFRDRHANCNFSM4YBCRXNA.