Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DIscover and print all information #84

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 17 additions & 10 deletions obd/elm327.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ def auto_protocol(self):
def set_baudrate(self, baud):
if baud is None:
# when connecting to pseudo terminal, don't bother with auto baud
if self.port_name().startswith("/dev/pts"):
if self.get_port_name().startswith("/dev/pts"):
logger.debug("Detected pseudo terminal, skipping baudrate setup")
return True
else:
Expand Down Expand Up @@ -330,29 +330,36 @@ def __error(self, msg):
logger.error(str(msg))


def port_name(self):
def get_port_name(self):
if self.__port is not None:
return self.__port.portstr
else:
return ""


def status(self):
return self.__status


def ecus(self):
return self.__protocol.ecu_map.values()
def get_port_baudrate(self):
if self.__port is not None:
return self.__port.baudrate
else:
return ""


def protocol_name(self):
def get_protocol_name(self):
return self.__protocol.ELM_NAME


def protocol_id(self):
def get_protocol_id(self):
return self.__protocol.ELM_ID


def get_ecus(self):
return self.__protocol.ecu_map.values()


def status(self):
return self.__status


def close(self):
"""
Resets the device, and sets all
Expand Down
71 changes: 55 additions & 16 deletions obd/obd.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,40 +155,46 @@ def status(self):
return self.interface.status()


# not sure how useful this would be

# def ecus(self):
# """ returns a list of ECUs in the vehicle """
# if self.interface is None:
# return []
# else:
# return self.interface.ecus()
def get_ecus(self):
""" returns a list of ECUs in the vehicle """
if self.interface is None:
return []
else:
return self.interface.get_ecus()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a specific use for this function? I couldn't quite justify yet, since we only really label the tx ID of the engine.



def protocol_name(self):
def get_protocol_name(self):
""" returns the name of the protocol being used by the ELM327 """
if self.interface is None:
return ""
else:
return self.interface.protocol_name()
return self.interface.get_protocol_name()


def protocol_id(self):
def get_protocol_id(self):
""" returns the ID of the protocol being used by the ELM327 """
if self.interface is None:
return ""
else:
return self.interface.protocol_id()
return self.interface.get_protocol_id()


def port_name(self):
def get_port_name(self):
""" Returns the name of the currently connected port """
if self.interface is not None:
return self.interface.port_name()
return self.interface.get_port_name()
else:
return ""


def get_port_baudrate(self):
""" Returns the speed of the currently connected port """
if self.interface is not None:
return str(self.interface.get_port_baudrate())
else:
return ""


def is_connected(self):
"""
Returns a boolean for whether a connection with the car was made.
Expand All @@ -207,13 +213,46 @@ def print_commands(self):
for c in self.supported_commands:
print(str(c))


def print_discovered(self):
"""
Utility function meant to print all information discovered:
protocol, port name, port baudrate and all supported commands.
"""
if self.interface is not None:
print ("The following settings were used to connect to the ECU:")
print ("Protocole: " + self.get_protocol_name())
print ("Port name: " + self.get_port_name())
print ("Port rate: " + self.get_port_baudrate())
print ("The following OBD commands are supported:")

_mylist=[]
for c in self.supported_commands:
_mylist.append(str(c))
_mylist.sort()
for i in _mylist:
print (i)

else:
print ("Impossible to print discovered information: no connection to the ECU.")

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, thanks for the PR!

While I don't have a problem with the printing code, I don't think the library core is the right place for this. I'd like to separate the concerns of OBD communications and tool-like terminal printing. I've been meaning to remove the print_commands function above (which I've intentionally not documented). Instead, I'd be totally supportive of adding a /tools directory for CLI applications akin to this.

def supports(self, cmd):
"""
Returns a boolean for whether the given command
is supported by the car
"""
return cmd in self.supported_commands


def get_supported_commands(self):
"""
Returns a list of commands
supported by the car
"""

if self.interface is not None:
return self.supported_commands
else:
return []


def test_cmd(self, cmd, warn=True):
Expand All @@ -228,7 +267,7 @@ def test_cmd(self, cmd, warn=True):
return False

# mode 06 is only implemented for the CAN protocols
if cmd.mode == 6 and self.interface.protocol_id() not in ["6", "7", "8", "9"]:
if cmd.mode == 6 and self.interface.get_protocol_id() not in ["6", "7", "8", "9"]:
if warn:
logger.warning("Mode 06 commands are only supported over CAN protocols")
return False
Expand Down
18 changes: 9 additions & 9 deletions tests/test_OBD.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def __init__(self, portname, UNUSED_baudrate=None, UNUSED_protocol=None):
self._status = OBDStatus.CAR_CONNECTED
self._last_command = None

def port_name(self):
def get_port_name(self):
return self._portname

def status(self):
Expand All @@ -32,10 +32,10 @@ def status(self):
def ecus(self):
return [ ECU.ENGINE, ECU.UNKNOWN ]

def protocol_name(self):
def get_protocol_name(self):
return "ISO 15765-4 (CAN 11/500)"

def protocol_id(self):
def get_protocol_id(self):
return "6"

def close(self):
Expand Down Expand Up @@ -122,30 +122,30 @@ def test_port_name():
"""
o = obd.OBD("/dev/null")
o.interface = FakeELM("/dev/null")
assert o.port_name() == o.interface._portname
assert o.get_port_name() == o.interface.get_port_name()

o.interface = FakeELM("A different port name")
assert o.port_name() == o.interface._portname
assert o.get_port_name() == o.interface.get_port_name()
Copy link
Owner

@brendan-w brendan-w Jul 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose it just comes down to preference; I'd prefer to leave these the way they were. I'd argue shorter is usually better. Also, if we decide to change them, lets do that in a separate PR.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shorter is never better unless there is a concern for memory, which there is not. More descriptive is better IMO.



def test_protocol_name():
o = obd.OBD("/dev/null")

o.interface = None
assert o.protocol_name() == ""
assert o.get_protocol_name() == ""

o.interface = FakeELM("/dev/null")
assert o.protocol_name() == o.interface.protocol_name()
assert o.get_protocol_name() == o.interface.get_protocol_name()


def test_protocol_id():
o = obd.OBD("/dev/null")

o.interface = None
assert o.protocol_id() == ""
assert o.get_protocol_id() == ""

o.interface = FakeELM("/dev/null")
assert o.protocol_id() == o.interface.protocol_id()
assert o.get_protocol_id() == o.interface.get_protocol_id()



Expand Down