Skip to content

Conversation

@Riley-King
Copy link
Contributor

The command *SAV <X> takes ~50 ms on the SPD3303X but we only wait for ~21 ms before trying to use it which causes it to crash and needs to be power cycled before it will work again.

Changes;

  • Added a slow write function.
  • save.* now uses the slow write function.

@Riley-King
Copy link
Contributor Author

@Jasper-Harvey0 The *WAI command can not be used as the SPD3303X has not implemented it.

SPD3303X manual

@Riley-King
Copy link
Contributor Author

Changed to using lambda+kwarg to introduce the delay.
The delay for saving groups was expanded to 100 ms because these pps' are crap and was not consistently passing tests.

1 failed, 29 passed, 4 xfailed, 1 warning in 41.51s
The failure is a result of tolerance issues, not any fault of the driver;
FAILED test/drivers/test_siglent_spd_3303X.py::test_measure_voltage[2.2-channel2-CH2] - assert 2.212 == 2.2 ± 0.01

@Riley-King Riley-King mentioned this pull request Sep 18, 2025
8 tasks
@clint-lawrence
Copy link
Collaborator

The failure is a result of tolerance issues, not any fault of the driver;
FAILED test/drivers/test_siglent_spd_3303X.py::test_measure_voltage[2.2-channel2-CH2] - assert 2.212 == 2.2 ± 0.01

That tolerance is tighter than the datasheet spec for the instrument, so make sense to adjust it. +/- 50 mV is probably a reasonable spot to leave it

Riley King added 2 commits December 18, 2025 11:35
…t e.g; `while pps.ch1.voltage() != 0:...`
…amming it e.g; `while pps.ch1.voltage() != 0:...`"

This actually doesn't do anything
VISA has a builtin delay set by default for all query's to 100 msec.

This reverts commit 481b77a.
@daniel-montanari
Copy link
Collaborator

https://pyvisa.readthedocs.io/en/latest/introduction/communication.html#getting-the-instrument-configuration-right

https://pyvisa.readthedocs.io/en/latest/api/resources.html#pyvisa.resources.SerialInstrument.query_binary_values
We use query_ascii_values query and write
The write command doesn't use query_delay so adding manual sleeps makes sense to stop spamming.
The other commands shouldn't need manual sleeps as they should already default to sleeping for 100ms. If someone has overridden the default then I think it is on them for setting the delay too short.

@Riley-King
Copy link
Contributor Author

Riley-King commented Dec 18, 2025

Why are the tests failing here? It is testing code that has already passed testing back in September. Have the tests changed between now and then?

It appears that the test environment has not installed the cmd2.ansi module.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants