slaclab / pysmurf

Other
2 stars 9 forks source link

Setup function leaves streaming on #652

Open jlashner opened 3 years ago

jlashner commented 3 years ago

Describe the bug

During S.setup() pysmurf sets S.set_stream_enable(1) to start a stream but never closes it. Because all this data is written to G3 files, this means data will be written indefinitely unless someone turns it off.

agustiner commented 2 years ago

Easy to change. But my only concern is if some people expect this to be on by default.

https://github.com/slaclab/pysmurf/blob/41e41c9c568aecd7e350ff2f050be58498c56af9/python/pysmurf/client/base/smurf_control.py#L609

agustiner commented 2 years ago

After poking around, this feels more logical. I believe the correct way to start streaming data from pysmurf is with stream_data_on(), not set_stream_enable(1). So it feels like a bug that set_stream_enable(1) is in there. In fact, it might be nice to remove set_stream_enable to force usage of stream_data_on, but that's probably not backwards compatible.

Places that are now redundant:

https://github.com/simonsobs/sodetlib/blob/1ff24c68ef43c055a71948c9358b7e40991839d8/scripts/setup_pysmurf.py#L10

https://github.com/slaclab/pysmurf/blob/0fef2dda87e6da292485266b3bf630c9b7ca97dc/scratch/shawn/thermal_test.py#L287

Anywhere with disable_streaming=true: https://github.com/slaclab/pysmurf/search?q=disable_streaming

Places that should enable streaming and already do:

https://github.com/slaclab/pysmurf/blob/e12b783a829129936840010d0cf8ee5a9defb95d/python/pysmurf/client/util/smurf_util.py#L957

Anywhere with stream_data_on: https://github.com/slaclab/pysmurf/search?q=stream_data_on

Places that should enable streaming but do not:

https://github.com/slaclab/pysmurf/blob/b95a6dcb40629a0e16a16318e819960b0bf01c7b/cfg_files/rflab/smurf_startup_6carrier.cfg#L14

Anywhere without stream_data_on: ???. This is the only concern. Is there anywhere that assumes streaming after setup().