Fixed issue where metadata would come with garbage

[Tjoms99]

Investigated the PR/issue discussed here #49.

Turns out not properly exiting a program running the PPK2 influences the next PPK2 session. Setting up the metadata the first time will throw an exception due to the garbage data (from the previous session) that comes a long with it. Physically turning the PPK2 on and off using the switch resolves this issue, but that’s inconvenient to do every time; So I’ve added a software fix here :)

To reproduce the issue and showcase the fix

1. Connect the PPK2 via USB

2. Open a terminal and run the example

python3 examples/example_measure_mw.py

3. Terminate the program (2x)

ctrl + c

4. Run the example again and you'll probably see this in the terminal

Attempt 1/2 - Failed to get valid PPK2 metadata: Could not retrieve valid metadata from the device.
Attempt 2/2 - Got metadata from PPK2
Initialized Power Profiler

This is where the UnicodeDecodeError would previously be triggered

[RmThrt]

Thanks for the fix, it helped to figure out the way my test work! Good fix for me :)

[MrKevinWeiss]

Also tested and it works for me!

[MrKevinWeiss]

This is on 4.2.0 and 4.2.1 fw.

[mstixenberger]

works fine, thank you!

[bobatsar]

Works also on my side. @wlgrd is it possible to merge it into master?

[tphillips-ict]

+1 Can we merge this? Running into the same issue.

Is this project still actively maintained?

[MrKevinWeiss]

A few small things, but at least now we can move!

[MrKevinWeiss]

Suggested change

	def get_modifiers(self, retries=2):
	def get_modifiers(self, retries=4):

I have seen this occurring in our devices, maybe a bit of extra time would help?

[MrKevinWeiss]

Suggested change

	for attempt in range(1, retries + 1):
	# Send command to request metadata
	self._write_serial((PPK2_Command.GET_META_DATA, ))
	try:
	metadata = self._read_metadata()
	ret = self._parse_metadata(metadata)
	print(f"Attempt {attempt}/{retries} - Got metadata from PPK2")
	return ret
	except ValueError as e:
	print(f"Attempt {attempt}/{retries} - Failed to get valid PPK2 metadata: {e}")
	# If this wasn't the last attempt, we try again by sending GET_META_DATA again.
	print("Failed to get modifiers after multiple attempts.")
	return None
	for attempt in range(1, retries + 1):
	# Send command to request metadata
	self._write_serial((PPK2_Command.GET_META_DATA, ))
	try:
	metadata = self._read_metadata()
	ret = self._parse_metadata(metadata)
	logging.info(f"Attempt {attempt}/{retries} - Got metadata from PPK2")
	return ret
	except ValueError as e:
	logging.warning(f"Attempt {attempt}/{retries} - Failed to get valid PPK2 metadata: {e}")
	# Wait a bit in case device was previously running
	time.sleep(0.5)
	# Clear anything in the buffer
	self.ser.read(self.ser.in_waiting)
	# If this wasn't the last attempt, we try again by sending GET_META_DATA again.
	raise IOError("Failed to get modifiers after multiple attempts. Reconnect device if this persists.")

I would try not to use prints here and rather logging to keep the stdout clean, logging allows a bit more control from the application side.

I think also throwing a proper exception if the modifiers are not there is better, then the applications would run uncalibrated, causing worse problems in the future... I have seen it before.

I also suggested (but didn't test) a way to hopefully harden the recovery process...

[Tjoms99]

I think, in hindsight, that the best way to solve this problem is to parse the metadata out from the garbage data (as it is in there). It feels like its the proper way to do it anyways, even on "clean" runs + it becomes more robust. Then, i think we can also remove the retries.

If it fails, raising an IOError is a very good suggestion!

I dont have time to look this before the weekend, but i would like to hear your thoughts on the suggestion above before continuing.

[MrKevinWeiss]

I haven't closely looked into the output or the complexity of the parsing... If you think it is easy enough (and robust across the various PPK versions) then we can give it a try. Solving hardware problems at the root is always better than adding retries and timeouts (though not always possible).

I am happy to test across firmware versions if you think you have it. Otherwise, I have implemented this patch (or close enough to this patch) and am running tests with it.