Skip to content

Instantly share code, notes, and snippets.

@qzed
Last active April 23, 2021 16:10
Show Gist options
  • Star 11 You must be signed in to star a gist
  • Fork 1 You must be signed in to fork a gist
  • Save qzed/01a93568efb863f1b7887f0f375c03fc to your computer and use it in GitHub Desktop.
Save qzed/01a93568efb863f1b7887f0f375c03fc to your computer and use it in GitHub Desktop.
Surface Book 2 / Surface Pro (2017) / Surface Laptop UART protocol proof-of-concept script.
#!/usr/bin/env python3
import json
import crcmod
from argparse import ArgumentParser
from pathlib import Path
import serial
from serial import Serial
DEFAULT_DEVICE = '/dev/ttyS4'
DEFAULT_BAUD_RATE = 3000000
CRC_FN = crcmod.predefined.mkCrcFun('crc-ccitt-false')
def setup_device(port, baudrate):
# definition from DSDT
return Serial(
port=port,
baudrate=baudrate,
bytesize=serial.EIGHTBITS,
parity=serial.PARITY_NONE,
stopbits=serial.STOPBITS_ONE,
rtscts=False,
dsrdtr=False,
timeout=0,
)
def crc(pld):
x = CRC_FN(bytes(pld))
return [x & 0xff, (x >> 0x08) & 0xff]
def to_int(bytes):
return int.from_bytes(bytes, byteorder='little')
class Counters:
PATH = Path(__file__).parent / '.counters.json'
@staticmethod
def load():
if Counters.PATH.is_file():
with open(Counters.PATH) as fd:
data = json.load(fd)
seq = data['seq']
cnt = data['cnt']
else:
seq = 0x00
cnt = 0x0000
return Counters(seq, cnt)
def __init__(self, seq, cnt):
self.seq = seq
self.cnt = cnt
def store(self):
with open(Counters.PATH, 'w') as fd:
data = {'seq': self.seq, 'cnt': self.cnt}
json.dump(data, fd)
def inc_seq(self):
self.seq = (self.seq + 1) & 0xFF
def inc_cnt(self):
self.cnt = (self.cnt + 1) & 0xFFFF
def inc(self):
self.inc_seq()
self.inc_cnt()
class Command:
def __init__(self, rtc, riid, rcid, rsnc=0x01, quiet=False):
self.rtc = rtc
self.riid = riid
self.rcid = rcid
self.rsnc = rsnc
self.quiet = quiet
def _write_msg(self, dev, seq, cnt):
cnt_lo = cnt & 0xff
cnt_hi = (cnt >> 0x08) & 0xff
hdr = [0x80, 0x08, 0x00, seq]
pld = [0x80, self.rtc, 0x01, 0x00, self.riid, cnt_lo, cnt_hi, self.rcid]
msg = [0xaa, 0x55] + hdr + crc(hdr) + pld + crc(pld)
return dev.write(bytes(msg))
def _write_ack(self, dev, seq):
hdr = [0x40, 0x00, 0x00, seq]
msg = [0xaa, 0x55] + hdr + crc(hdr) + [0xff, 0xff]
return dev.write(bytes(msg))
def _read_ack(self, dev, exp_seq):
msg = bytes()
while len(msg) < 0x0A:
msg += dev.read(0x0A - len(msg))
if not self.quiet:
print("received: {}".format(msg.hex()))
assert msg[0:2] == bytes([0xaa, 0x55])
assert msg[3:5] == bytes([0x00, 0x00])
assert msg[6:8] == bytes(crc(msg[2:-4]))
assert msg[8:] == bytes([0xff, 0xff])
mty = msg[2]
seq = msg[5]
if mty == 0x40:
assert seq == exp_seq
return mty == 0x04
def _read_msg(self, dev, cnt):
cnt_lo = cnt & 0xff
cnt_hi = (cnt >> 0x08) & 0xff
buf = bytes()
rem = 0x08 # begin with header length
while len(buf) < rem:
buf += dev.read(0x0400)
# if we got a header, validate it
if rem == 0x08 and len(buf) >= 0x08:
hdr = buf[0:8]
assert hdr[0:3] == bytes([0xaa, 0x55, 0x80])
assert hdr[-2:] == bytes(crc(hdr[2:-2]))
rem += hdr[3] + 10 # len(payload) + frame + crc
hdr = buf[0:8]
msg = buf[8:hdr[3]+10]
rem = buf[hdr[3]+10:]
if not self.quiet:
print("received: {}".format(hdr.hex()))
print("received: {}".format(msg.hex()))
assert msg[0:8] == bytes([0x80, self.rtc, 0x00, 0x01, self.riid, cnt_lo, cnt_hi, self.rcid])
assert msg[-2:] == bytes(crc(msg[:-2]))
seq = hdr[5]
pld = msg[8:-2]
return seq, pld, rem
def _read_clean(self, dev, buf=bytes()):
buf += dev.read(0x0400) # make sure we're not missing some bytes
while buf:
# get header / detect message type
if len(buf) >= 0x08:
if buf[0:3] == bytes([0xaa, 0x55, 0x40]): # ACK
while len(buf) < 0x0A:
buf += dev.read(0x0400)
if not self.quiet:
print("ignored ACK: {}".format(buf[:0x0a].hex()))
buf = bytes(buf[0x0a:])
elif buf[0:3] == bytes([0xaa, 0x55, 0x80]): # response
buflen = 0x0a + buf[3]
while len(buf) < buflen:
buf += dev.read(0x0400)
if not self.quiet:
print("ignored MSG: {}".format(buf[:buflen].hex()))
buf = bytes(buf[buflen:])
elif buf[0:3] == bytes([0x4e, 0x00, 0x53]): # control message?
while len(buf) < 0x19:
buf += dev.read(0x0400)
if not self.quiet:
print("ignored CTRL: {}".format(buf[:0x19].hex()))
buf = bytes(buf[0x19:])
else: # unknown
if not self.quiet:
print("ignored unknown: {}".format(buf.hex()))
assert False
buf += dev.read(0x0400)
def run(self, dev, cnt):
self._read_clean(dev)
self._write_msg(dev, cnt.seq, cnt.cnt)
retry = self._read_ack(dev, cnt.seq)
# retry one time on com failure
if retry:
self._write_msg(dev, cnt.seq, cnt.cnt)
retry = self._read_ack(dev, cnt.seq)
if retry:
if not self.quiet:
print('Communication failure: invalid ACK, try again')
return
try:
if self.rsnc:
seq, pld, rem = self._read_msg(dev, cnt.cnt)
self._write_ack(dev, seq)
else:
seq, pld, rem = 0, bytes(), bytes()
self._read_clean(dev, rem)
finally:
cnt.inc()
return self._handle_payload(pld)
def _handle_payload(self, pld):
return None
class Gbos(Command):
def __init__(self, **kwargs):
super().__init__(0x11, 0x00, 0x0d, **kwargs)
def _handle_payload(self, pld):
return {
'Base Status': hex(pld[0]),
}
class Psr(Command):
def __init__(self, bat, **kwargs):
super().__init__(0x02, bat, 0x0d, **kwargs)
def _handle_payload(self, pld):
return {
'Power Source': hex(to_int(pld[0:4])),
}
class Sta(Command):
def __init__(self, bat, **kwargs):
super().__init__(0x02, bat, 0x01, **kwargs)
def _handle_payload(self, pld):
return {
'Battery Status': hex(to_int(pld[0:4])),
}
class Bst(Command):
def __init__(self, bat, **kwargs):
super().__init__(0x02, bat, 0x03, **kwargs)
def _handle_payload(self, pld):
return {
'State': hex(to_int(pld[0:4])),
'Present Rate': hex(to_int(pld[4:8])),
'Remaining Capacity': hex(to_int(pld[8:12])),
'Present Voltage': hex(to_int(pld[12:16])),
}
class Bix(Command):
def __init__(self, bat, **kwargs):
super().__init__(0x02, bat, 0x02, **kwargs)
def _handle_payload(self, pld):
return {
'Revision': hex(pld[0]),
'Power Unit': hex(to_int(pld[1:5])),
'Design Capacity': hex(to_int(pld[5:9])),
'Last Full Charge Capacity': hex(to_int(pld[9:13])),
'Technology': hex(to_int(pld[13:17])),
'Design Voltage': hex(to_int(pld[17:21])),
'Design Capacity of Warning': hex(to_int(pld[21:25])),
'Design Capacity of Low': hex(to_int(pld[25:29])),
'Cycle Count': hex(to_int(pld[29:33])),
'Measurement Accuracy': hex(to_int(pld[33:37])),
'Max Sampling Time': hex(to_int(pld[37:41])),
'Min Sampling Time': hex(to_int(pld[41:45])),
'Max Averaging Interval': hex(to_int(pld[45:49])),
'Min Averaging Interval': hex(to_int(pld[49:53])),
'Capacity Granularity 1': hex(to_int(pld[53:57])),
'Capacity Granularity 2': hex(to_int(pld[57:61])),
'Model Number': pld[61:82].decode().rstrip('\0'),
'Serial Number': pld[82:93].decode().rstrip('\0'),
'Type': pld[93:98].decode().rstrip('\0'),
'OEM Information': pld[98:119].decode().rstrip('\0'),
}
class PrettyBat:
def __init__(self, bat, **kwargs):
self.bix = Bix(bat, **kwargs)
self.bst = Bst(bat, **kwargs)
def run(self, dev, cnt):
bix = self.bix.run(dev, cnt)
bst = self.bst.run(dev, cnt)
state = int(bst['State'], 0)
vol = int(bst['Present Voltage'], 0)
rem_cap = int(bst['Remaining Capacity'], 0)
full_cap = int(bix['Last Full Charge Capacity'], 0)
rate = int(bst['Present Rate'], 0)
bat_states = {
0: 'None',
1: 'Discharging',
2: 'Charging',
4: 'Critical',
5: 'Critical (Discharging)',
6: 'Critical (Charging)',
}
bat_state = bat_states[state]
bat_vol = vol / 1000
if full_cap <= 0:
bat_rem_perc = '<unavailable>'
else:
bat_rem_perc = "{}%".format(int(rem_cap / full_cap * 100))
if state == 0x00 or rate == 0:
bat_rem_life = '<unavailable>'
else:
bat_rem_life = "{:.2f}h".format(rem_cap / rate)
return {
'State': bat_state,
'Voltage': "{}V".format(bat_vol),
'Percentage': bat_rem_perc,
'Remaing ': bat_rem_life,
}
class BaseLock(Command):
def __init__(self, lock, **kwargs):
super().__init__(0x11, 0x00, 0x06 if lock else 0x07, 0x00, **kwargs)
COMMANDS = {
'lid0.gbos': (Gbos, ()),
'adp1._psr': (Psr, (0x01,)),
'bat1._sta': (Sta, (0x01,)),
'bat1._bst': (Bst, (0x01,)),
'bat1._bix': (Bix, (0x01,)),
'bat2._sta': (Sta, (0x02,)),
'bat2._bst': (Bst, (0x02,)),
'bat2._bix': (Bix, (0x02,)),
'bat1.pretty': (PrettyBat, (0x01,)),
'bat2.pretty': (PrettyBat, (0x02,)),
'base.lock': (BaseLock, (True,)),
'base.unlock': (BaseLock, (False,)),
}
def main():
cli = ArgumentParser(description='Surface Book 2 / Surface Pro (2017) embedded controller requests.')
cli.add_argument('-d', '--device', default=DEFAULT_DEVICE, metavar='DEV', help='the UART device')
cli.add_argument('-b', '--baud', default=DEFAULT_BAUD_RATE, type=lambda x: int(x, 0), metavar='BAUD', help='the baud rate')
cli.add_argument('-c', '--cnt', type=lambda x: int(x, 0), help='overwrite CNT')
cli.add_argument('-s', '--seq', type=lambda x: int(x, 0), help='overwrite SEQ')
cli.add_argument('-q', '--quiet', action='store_true', help='do not print debug messages, just results')
commands = cli.add_subparsers()
for cmd in COMMANDS.keys():
parser = commands.add_parser(cmd, help="run request '{}'".format(cmd.upper()))
parser.set_defaults(command=cmd)
args = cli.parse_args()
dev = setup_device(args.device, args.baud)
cmd = COMMANDS.get(args.command)
cmd = cmd[0](*cmd[1], quiet=args.quiet)
cnt = Counters.load()
if args.seq is not None:
cnt.seq = args.seq
if args.cnt is not None:
cnt.cnt = args.cnt
try:
res = cmd.run(dev, cnt)
if args.quiet:
for k, v in sorted(res.items()):
print("{}: {}".format(k, v))
else:
import pprint
print()
pprint.pprint(res)
finally:
cnt.store()
if __name__ == '__main__':
main()
[[source]]
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"
[packages]
crcmod = "*"
pyserial = "*"
[dev-packages]
pylint = "*"
@qzed
Copy link
Author

qzed commented Sep 26, 2018

Either way, properly bumping the sequence makes sure we're not going to get that issue in the driver.

@bpasteur
Copy link

Apparently the serial interface is not very open to drivers using the device other than the standard serial driver, which is why the serdev interface was introduced:
http://events17.linuxfoundation.org/sites/events/files/slides/serdev-elce-2017-2.pdf

While a serdev driver looks like the way to go there is however a way to do it outside of the serdev interface using a Line-Discipline driver which requires a user space daemon to open the port and keep it open. Not sure how a Line-Discipline works but if the driver maps in the OR01 region as implemented in the I2C driver written by Benjamin Tissoires and fills it in with the bst and bix data from the UART and sends out the ACPI notifications the built in adapter and battery modules should just work. My guess is the driver would have to set up a polling thread to do this.

@qzed
Copy link
Author

qzed commented Sep 26, 2018

I agree, a serdev driver seems like the proper way to go. Additionally we need to install the address space handler for OR01, but I've already got some code for that working. The address space handler should then communicate with the UART. I'm not sure yet if we should split that into two sub-drivers, one for the UART (MSHW0084) and one for the address-space handler (MSHW0091), which then communicate with each other. For me, this currently makes the most sense.

The only issue I still have is that I don't know why I can't get a serdev driver to probe, or rather what I'm doing wrong there. It might take me some time to get this working.

With regards to the line-discipline: As far as I understood that, it's basically equal to a protocol. Aside from the rodeo through the user-space that seems to be necessary for this approach, it looks like the kernel is heavily shifting to the serdev model (because of that), so if we ever want to push this upstream, I think we have to go with the serdev driver.

@qzed
Copy link
Author

qzed commented Sep 26, 2018

I've updated the script so it's a bit easier to use and adaptable for future debugging/testing.

Also I've added automatic updates to the counters by storing them in a .counters.json file (in the same directory as the script).

@qzed
Copy link
Author

qzed commented Sep 26, 2018

I still don't know what the secondary counter (cnt in the script) is for. Has any of you guys any idea? It does seem to work without incrementing it, but according to the logs of @carrylook, it is also incremented.

@tworzenieweb
Copy link

tworzenieweb commented Sep 27, 2018

python3 battery.py -d /dev/ttyS0 lid0.gbos

received: aa5504000000314e
received: ffff
received: aa55400000005cea
received: ffff
received: aa5580090049841e
received: 801100010000000d017afe

{'Base Status': '0x1'}

python3 battery.py -d /dev/ttyS0 adp1._psr

received: aa5504000000314e
received: ffff
received: aa55400000021eca
received: ffff
received: aa55800c004a17c5
received: 800200010102000d000000008263

{'Power Source': '0x0'}

python3 battery.py -d /dev/ttyS0 bat1._bst

received: aa5540000004d8aa
received: ffff
received: aa558018004b954a
received: 800200010104000301000000b50000005848000046200000d29f

{'Battery Present Rate': '0xb5',
 'Battery Present Voltage': '0x2046',
 'Battery Remaining Capacity': '0x4858',
 'Battery State': '0x1'}

python3 battery.py -d /dev/ttyS0 bat1._bix

received: aa55400000069a8a
received: ffff
received: aa55807f004c8924
received: 80020001010600020000000000a05a00006e5a000001000000921d0000000000000000000053000000e8030000ffffffffffffffffe8030000e80300000a0000000a0000004d313030353034360000000000000000000000000030303034303436383031004c494f4e0044594e00000000000000000000000000000000000068a0

{'Battery Capacity Granularity 1': '0xa',
 'Battery Capacity Granularity 2': '0xa',
 'Battery Technology': '0x1',
 'Battery Type': 'LION',
 'Cycle Count': '0x53',
 'Design Capacity': '0x5aa0',
 'Design Capacity of Low': '0x0',
 'Design Capacity of Warning': '0x0',
 'Design Voltage': '0x1d92',
 'Last Full Charge Capacity': '0x5a6e',
 'Max Averaging Interval': '0x3e8',
 'Max Sampling Time': '0xffffffff',
 'Measurement Accuracy': '0x3e8',
 'Min Averaging Interval': '0x3e8',
 'Min Sampling Time': '0xffffffff',
 'Model Number': 'M1005046',
 'OEM Information': 'DYN',
 'Power Unit': '0x0',
 'Revision': '0x0',
 'Serial Number': '0004046801'}

python3 battery.py -d /dev/ttyS0 bat2._bst

received: aa5504000000314e
received: ffff
received: aa5540000008546b
received: ffff
received: aa558018004d532a
received: 8002000102080003010000008b430000f8b60000542e0000581a

{'Battery Present Rate': '0x438b',
 'Battery Present Voltage': '0x2e54',
 'Battery Remaining Capacity': '0xb6f8',
 'Battery State': '0x1'}

python3 battery.py -d /dev/ttyS0 bat2._bix

received: aa5504000000314e
received: ffff
received: aa554000000a164b
received: ffff
received: aa0055807f004ecb
Traceback (most recent call last):
  File "battery.py", line 269, in <module>
    main()
  File "battery.py", line 264, in main
    cmd.run(dev, cnt)
  File "battery.py", line 155, in run
    seq, pld = self._read_msg(dev, cnt.cnt)
  File "battery.py", line 127, in _read_msg
    assert hdr[0:3] == bytes([0xaa, 0x55, 0x80])
AssertionError

I got SB2 15"

[ 2.083825] dw-apb-uart.4: ttyS0 at MMIO 0xd4748000 (irq = 20, base_baud = 3000000) is a 16550A

@qzed
Copy link
Author

qzed commented Sep 27, 2018

The aa 00 55 ... response is interesting. I haven't seen that before, but after playing around I can see that happen some times on the _BIX request. Also I can get some more weird responses:

  • aa0055807f00XXXX (as seen above, where XXXX is variable),
  • aa005500807f00XX,
  • 0000e8030000ffff, and
  • 0a00e8030000ffff.

The last two are pretty rare. The first does seem to happen from time to time. I think the reason I can only observe this on the _BIX request is due to the length of that response. It does look a bit like the chip gets confused sometimes. Re-trying with a new sequence-id seems to work (as the old one has been acknowledged at the point of failure).

I've updated the script to also update the counters when the response is faulty. This way we can just re-run the script on failure.

@bpasteur
Copy link

As a temporary work around I'm trying to use this script and I'm also seeing the aa 00 55 ... response, only on the bix request so far but on my system it appears to happen at least 50% of the time. I wonder if it has to do with reading the data in chunks. It may be a driver bug or a quirky chip, perhaps if you read all available data at once it would fix the issue? Since the bix data is for the most part fixed for now I can cache it and use the cache when the command fails.

Out of curiosity I made a quick change to the prints and ran the script using python 2.7. Interestingly it hung on the first read just like I was seeing in my initial attempts to talk to the UART. Any idea what might be going on there?

@qzed
Copy link
Author

qzed commented Sep 28, 2018

@bpasteur Thanks for reminding me of that! The blocking is actually another part I should look at. I tried to use the python os module at first, but it was also blocking, so I decided to just try out the pyserial module, which ended up working. I think this is something with respect to opening the connection (pyserial also uses os internally). The settings (baud-rate etc.) should already be applied through ACPI. I'll try to have a look at it later.

As for reading the data in chunks: I mainly did that because it was easier to work with, knowing the exact lengths. But I guess we could try reading everything at once. A quick glance at the pyserial API documentation suggests setting timeout = 0 in the Serial constructor should do the trick. Serial.read expects the number of bytes to be read as parameter, but setting this to 512 should work (the maximum response size is, including ACK probably only slightly larger than 256 bytes).

@qzed
Copy link
Author

qzed commented Sep 28, 2018

Ok, so after a quick look it's not just some open-flags that have to be set. There seems to be some configuration required on the tty-port. I'm not going to look into this any further at the moment, but it might be a thing to keep in mind if we experience something similar in the driver.

@qzed
Copy link
Author

qzed commented Sep 29, 2018

Okay, I've changed the read strategy. I can't see any invalid packages any more, but there's some other interesting stuff: I can now see two identical packages being returned, and occasionally a third smaller one starting with 4e 00 53 4d 50 then some zeros and changing last two bytes. Maybe the device is getting a bit impatient and re-sends the package due to it not having been acknowledged yet? But I've got absolutely no idea what the third one is for, or if it even is something valid (due to the other bytes being constant, the last two bytes are definitely not a checksum of it, or at least it alone).

@qzed
Copy link
Author

qzed commented Sep 29, 2018

@bpasteur I've added a command to pretty-print the relevant battery stats.

@bpasteur
Copy link

@qzed thanks much, this is great. The old script is working for me now with some retry logic and bix caching but I'll give it a shot and see how it works.

@Catley94
Copy link

Hey, may i ask how I can use this for my SB2? I'd love the battery percentage work, even if it's through an app and it's a temporary fix until the driver.

@bpasteur
Copy link

@qzed, I tried the new script and it works great. I haven't see any failed commands and I was seeing about a 50% failure rate on the bix commands with the old script. As long as you go by the counts returned in the read data everything should just work and the rest is just throw away data. I wouldn't be surprised if the Windows driver is doing the same thing. If you look at the Windows raw logs the reads are sent out with a length of 536 bytes. Seems like an odd number, maybe that's the length of the device buffer, but every read I see uses that count.

@qzed
Copy link
Author

qzed commented Sep 30, 2018

@Catley94 It's just a basic python script that prints to the console, so nothing fancy. Download and run it with

python3 <path-to-script.py> bat1.pretty

for the status of the clipboard battery, or replace bat1 with bat2 for the base battery. You might need to install the dependencies (see first comment). To do this the easy way I'd recommend you install pipenv and also download the Pipfile (and the script) to a dedicated folder, then run pipenv install inside that folder and afterwards run the script with pipenv run (instead of python3). You can also specify -h` to see the other commands.

@qzed
Copy link
Author

qzed commented Sep 30, 2018

@bpasteur Good to see that! 536 indeed seems odd, I'd have to have a look at the ACPI and calculate the maximum payload size to see if it's somehow related to this. A quick guess would be that they assumed two responses (as we are seeing) with maximum payload size plus the size of the last weird message, plus relevant header/frame lengths. I think this could put it somewhere close to 536 bytes.

Unfortunately I'm a bit busy up to mid next week. I'll have another look at it then!

@Pimon1
Copy link

Pimon1 commented Oct 2, 2018

Thanks a lot @qzed! In the meantime, I was able to get the battery status visible in the status bar with grep, awk and Argos:

#!/bin/sh
sudo /path/to/mshw0084-rqst.py bat1.pretty | grep Precentage | awk -F"'" '{print $4}'

Be sure to add /path/to/mshw0084-rqst.py in visudo (so that it can be executed without password), write the lines above into ~/.config/argos/batterystatus.1m.sh and enjoy battery status!

@bpasteur
Copy link

bpasteur commented Oct 2, 2018

@qzed, I have been playing with your script a bit and modified it to send the adapter, bst, and bix commands off every 5 seconds. I also added retires to the commands when they failed. After a period of time I would get failures to the point of it no longer working and had to reboot to clear it up. I made some changes to read extra bytes off of the device after a failure and to my surprise at times I was seeing as many as 1000+ extra bytes read in. I was still getting failures but they would clear up and I could continue to run. Then I decided to add a _clear_extra_bytes(self, dev) function to the Command class, just a while True reading 536 bytes off the dev until no bytes came back. I called this function at the top of the Command run() function and if a failure occured, and virtually all of my failures went away. I am now only seeing 25 more or less extra bytes read in only occasionally at the top of the run() command. Hope this helps.

@qzed
Copy link
Author

qzed commented Oct 4, 2018

@bpasteur Interesting, have you tried to set the main read to something above 536 bytes? Also did you do this with a modified version of the script? If so you may have to call Serial.reset_input_buffer() in-between requests. This should basically do what you did manually.

Also sorry it took me so long to answer, I'm back working on this now.

@qzed
Copy link
Author

qzed commented Oct 4, 2018

@bpasteur I've updated the script to read (and try to decode) any remaining bytes in the buffer before and after each command (similar to what you've suggested). I've been running bat1.pretty and bat2.pretty for a while now in combination with the sched module (repeating every 5 seconds) and have not experienced any problems yet.

@bpasteur
Copy link

bpasteur commented Oct 4, 2018

@qzed not sure why you would try to decode the extra data. Once you send your request and process what you expected seems to me you should just throw away anything else, that;s what I have been doing. This may be something you need to do in the driver. This Linux Adapter driver that loads was not specifically written for this UART hardware and I believe loads as a 'hardware family' type driver. I suspect there are some issues with the hardware that need to be handled slightly differently is why we are seeing these extra bytes or perhaps it's timing issues that may go away if a driver is accessing this data.

As far as it taking you a long time to reply, completely understood and expected... we all have other obligations that take precedence

@qzed
Copy link
Author

qzed commented Oct 4, 2018

@bpasteur

not sure why you would try to decode the extra data

That is mainly to see what else is being sent, I'd like to get a good overview of this (and how the device behaves) before discarding stuff (for stability I agree that it's better to just discard it, e.g. by flushing the buffer as mentioned previously).

I've also been playing around with various timeouts between the request and acknowledgement of the response. So far I can reproduce some of the responses we've seen earlier (e.g. swapped bytes) by this. I also think that (at least the duplicated messages) are timing issues that should get resolved with the driver, simply due to the latency involved.

Interestingly I haven't been able to reproduce the 4e 00 53 4d 50 ... messages with the new read setup.

@JackMorganNZ
Copy link

Thanks for all your hard work on this script. Just to let you know "precentage" should be "percentage".

@qzed
Copy link
Author

qzed commented Oct 9, 2018

@JackMorganNZ Oops, thanks for catching that!

@sphh
Copy link

sphh commented Oct 12, 2018

Thanks @qzed for that great script!!

Since I want to use it in one of my own scripts, I took the liberty to add the command line switch -q|--quiet to remove the debugging output. I also changed the output in quiet mode to make it easier to parse (by not using pprint, thus removing the curly brackets). If you want to include it into your script, please feel free to do:

--- /home/sph/Software/sb2-batteryORIG.py
+++ /home/sph/Software/sb2-batteryNEW.py
@@ -1,7 +1,6 @@
 #!/usr/bin/env python3
 
 import json
-import pprint
 import crcmod
 
 from argparse import ArgumentParser
@@ -77,11 +76,12 @@
 
 
 class Command:
-    def __init__(self, rtc, riid, rcid, rsnc=0x01):
+    def __init__(self, rtc, riid, rcid, rsnc=0x01, quiet=False):
         self.rtc = rtc
         self.riid = riid
         self.rcid = rcid
         self.rsnc = rsnc
+        self.quiet = quiet
 
     def _write_msg(self, dev, seq, cnt):
         cnt_lo = cnt & 0xff
@@ -104,7 +104,8 @@
         while len(msg) < 0x0A:
             msg += dev.read(0x0A - len(msg))
 
-        print("received: {}".format(msg.hex()))
+        if not self.quiet:
+            print("received: {}".format(msg.hex()))
 
         assert msg[0:2] == bytes([0xaa, 0x55])
         assert msg[3:5] == bytes([0x00, 0x00])
@@ -141,8 +142,9 @@
         msg = buf[8:hdr[3]+10]
         rem = buf[hdr[3]+10:]
 
-        print("received: {}".format(hdr.hex()))
-        print("received: {}".format(msg.hex()))
+        if not self.quiet:
+            print("received: {}".format(hdr.hex()))
+            print("received: {}".format(msg.hex()))
 
         assert msg[0:8] == bytes([0x80, self.rtc, 0x00, 0x01, self.riid, cnt_lo, cnt_hi, self.rcid])
         assert msg[-2:] == bytes(crc(msg[:-2]))
@@ -162,7 +164,8 @@
                     while len(buf) < 0x0A:
                         buf += dev.read(0x0400)
 
-                    print("ignored ACK: {}".format(buf[:0x0a].hex()))
+                    if not self.quiet:
+                        print("ignored ACK: {}".format(buf[:0x0a].hex()))
                     buf = bytes(buf[0x0a:])
 
                 elif buf[0:3] == bytes([0xaa, 0x55, 0x80]):             # response
@@ -170,18 +173,21 @@
                     while len(buf) < buflen:
                         buf += dev.read(0x0400)
 
-                    print("ignored MSG: {}".format(buf[:buflen].hex()))
+                    if not self.quiet:
+                        print("ignored MSG: {}".format(buf[:buflen].hex()))
                     buf = bytes(buf[buflen:])
 
                 elif buf[0:3] == bytes([0x4e, 0x00, 0x53]):             # control message?
                     while len(buf) < 0x19:
                         buf += dev.read(0x0400)
 
-                    print("ignored CTRL: {}".format(buf[:0x19].hex()))
+                    if not self.quiet:
+                        print("ignored CTRL: {}".format(buf[:0x19].hex()))
                     buf = bytes(buf[0x19:])
 
                 else:                                                   # unknown
-                    print("ignored unknown: {}".format(buf.hex()))
+                    if not self.quiet:
+                        print("ignored unknown: {}".format(buf.hex()))
                     assert False
 
             buf += dev.read(0x0400)
@@ -197,7 +203,8 @@
             retry = self._read_ack(dev, cnt.seq)
 
             if retry:
-                print('Communication failure: invalid ACK, try again')
+                if not self.quiet:
+                    print('Communication failure: invalid ACK, try again')
                 return
 
         try:
@@ -218,8 +225,8 @@
 
 
 class Gbos(Command):
-    def __init__(self):
-        super().__init__(0x11, 0x00, 0x0d)
+    def __init__(self, **kwargs):
+        super().__init__(0x11, 0x00, 0x0d, **kwargs)
 
     def _handle_payload(self, pld):
         return {
@@ -228,8 +235,8 @@
 
 
 class Psr(Command):
-    def __init__(self, bat):
-        super().__init__(0x02, bat, 0x0d)
+    def __init__(self, bat, **kwargs):
+        super().__init__(0x02, bat, 0x0d, **kwargs)
 
     def _handle_payload(self, pld):
         return {
@@ -238,8 +245,8 @@
 
 
 class Sta(Command):
-    def __init__(self, bat):
-        super().__init__(0x02, bat, 0x01)
+    def __init__(self, bat, **kwargs):
+        super().__init__(0x02, bat, 0x01, **kwargs)
 
     def _handle_payload(self, pld):
         return {
@@ -248,8 +255,8 @@
 
 
 class Bst(Command):
-    def __init__(self, bat):
-        super().__init__(0x02, bat, 0x03)
+    def __init__(self, bat, **kwargs):
+        super().__init__(0x02, bat, 0x03, **kwargs)
 
     def _handle_payload(self, pld):
         return {
@@ -261,8 +268,8 @@
 
 
 class Bix(Command):
-    def __init__(self, bat):
-        super().__init__(0x02, bat, 0x02)
+    def __init__(self, bat, **kwargs):
+        super().__init__(0x02, bat, 0x02, **kwargs)
 
     def _handle_payload(self, pld):
         return {
@@ -290,9 +297,9 @@
 
 
 class PrettyBat:
-    def __init__(self, bat):
-        self.bix = Bix(bat)
-        self.bst = Bst(bat)
+    def __init__(self, bat, **kwargs):
+        self.bix = Bix(bat, **kwargs)
+        self.bst = Bst(bat, **kwargs)
 
     def run(self, dev, cnt):
         bix = self.bix.run(dev, cnt)
@@ -335,25 +342,25 @@
 
 
 class BaseLock(Command):
-    def __init__(self, lock):
-        super().__init__(0x11, 0x00, 0x06 if lock else 0x07, 0x00)
+    def __init__(self, lock, **kwargs):
+        super().__init__(0x11, 0x00, 0x06 if lock else 0x07, 0x00, **kwargs)
 
 
 COMMANDS = {
-    'lid0.gbos': Gbos(),
-    'adp1._psr': Psr(0x01),
-    'bat1._sta': Sta(0x01),
-    'bat1._bst': Bst(0x01),
-    'bat1._bix': Bix(0x01),
-    'bat2._sta': Sta(0x02),
-    'bat2._bst': Bst(0x02),
-    'bat2._bix': Bix(0x02),
-
-    'bat1.pretty': PrettyBat(0x01),
-    'bat2.pretty': PrettyBat(0x02),
-
-    'base.lock': BaseLock(True),
-    'base.unlock': BaseLock(False),
+    'lid0.gbos': (Gbos, ()),
+    'adp1._psr': (Psr, (0x01,)),
+    'bat1._sta': (Sta, (0x01,)),
+    'bat1._bst': (Bst, (0x01,)),
+    'bat1._bix': (Bix, (0x01,)),
+    'bat2._sta': (Sta, (0x02,)),
+    'bat2._bst': (Bst, (0x02,)),
+    'bat2._bix': (Bix, (0x02,)),
+
+    'bat1.pretty': (PrettyBat, (0x01,)),
+    'bat2.pretty': (PrettyBat, (0x02,)),
+
+    'base.lock': (BaseLock, (True,)),
+    'base.unlock': (BaseLock, (False,)),
 }
 
 
@@ -363,6 +370,7 @@
     cli.add_argument('-b', '--baud', default=DEFAULT_BAUD_RATE, type=lambda x: int(x, 0), metavar='BAUD', help='the baud rate')
     cli.add_argument('-c', '--cnt', type=lambda x: int(x, 0), help='overwrite CNT')
     cli.add_argument('-s', '--seq', type=lambda x: int(x, 0), help='overwrite SEQ')
+    cli.add_argument('-q', '--quiet', action='store_true', help='do not print debug messages, just results')
     commands = cli.add_subparsers()
 
     for cmd in COMMANDS.keys():
@@ -373,6 +381,7 @@
 
     dev = setup_device(args.device, args.baud)
     cmd = COMMANDS.get(args.command)
+    cmd = cmd[0](*cmd[1], quiet=args.quiet)
 
     cnt = Counters.load()
     if args.seq is not None:
@@ -383,8 +392,14 @@
     try:
         res = cmd.run(dev, cnt)
 
-        print()
-        pprint.pprint(res)
+        if args.quiet:
+            for k, v in sorted(res.items()):
+                print("{}: {}".format(k, v))
+        else:
+            import pprint
+            print()
+            pprint.pprint(res)
+
     finally:
         cnt.store()
 

@qzed
Copy link
Author

qzed commented Oct 15, 2018

@sphh seems like a good idea, patch applied, thanks!

@calvous
Copy link

calvous commented Oct 17, 2018

@qzed
just a little side note:
I kind of think that the the script is overreading its boundaries for the fields:

            'Revision': hex(pld[0]),
            'Power Unit': hex(to_int(pld[1:5])),
            'Design Capacity': hex(to_int(pld[5:9])),
            'Last Full Charge Capacity': hex(to_int(pld[9:13])),
            'Technology': hex(to_int(pld[13:17])),
            'Design Voltage': hex(to_int(pld[17:21])),
            'Design Capacity of Warning': hex(to_int(pld[21:25])),
'Design Capacity of Low': hex(to_int(pld[25:29])),

You start from 0, then from1 to 5, then 5 to 9, 9 to 13, and so on. Aren't you overlapping bytes here or is that done on purpose?
I also see lots of progress. I will work it in to the fakedev stuff (later tomorrow).

@qzed
Copy link
Author

qzed commented Oct 17, 2018

@calvous The last index is exclusive (while the first one is inclusive), so it actually gets bytes 1 to (inclusively) 4 for eg. the "Power Unit", if that is what you mean.

@calvous
Copy link

calvous commented Oct 17, 2018

@qzed
yes, that's it. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment