refactor download status API to live in InstrumentChannel, and separate UI concerns from data reporting concerns

Author: jwise

scopehal/DemoOscilloscope.cpp

@@ -401,9 +401,6 @@ uint64_t DemoOscilloscope::GetSampleDepth()
 void DemoOscilloscope::SetSampleDepth(uint64_t depth)
 {
 	m_depth = depth;
-	// Another way to let GetChannelDownloadState() API know when to show progress bar is to
-	// force its behaviour according to the current sample depth using SetShowChannelsDownloadProgress() method
-	// SetShowChannelsDownloadProgress(depth>=10000000L);
 }
 
 void DemoOscilloscope::SetSampleRate(uint64_t rate)
@@ -494,7 +491,7 @@ bool DemoOscilloscope::AcquireData()
 	if(!m_triggerArmed)
 		return false;
 
-	// Tell the GetChannelDownloadState() of Oscilloscope class that waveform download operation is starting
+	// prepare all channels to be 'about to download'
 	ChannelsDownloadStarted();
 
 	//cap waveform rate at 50 wfm/s to avoid saturating cpu
@@ -538,10 +535,11 @@ bool DemoOscilloscope::AcquireData()
 			continue;
 
 		// Lambda passed to generate waveform methods to update "download" percentage
-		auto updateProgress = [i,this](int percentage) 
+		auto updateProgress = [i,this](float progress)
 			{
-				this->UpdateChannelDownloadState(i,percentage);
+				this->ChannelsDownloadStatusUpdate(i, InstrumentChannel::DownloadState::DOWNLOAD_IN_PROGRESS, progress);
 			};
+		this->ChannelsDownloadStatusUpdate(i, InstrumentChannel::DownloadState::DOWNLOAD_IN_PROGRESS, 0.0);
 		switch(i)
 		{
 			case 0:
@@ -572,6 +570,7 @@ bool DemoOscilloscope::AcquireData()
 		}
 
 		waveforms[i]->MarkModifiedFromCpu();
+		this->ChannelsDownloadStatusUpdate(i, InstrumentChannel::DownloadState::DOWNLOAD_FINISHED, 1.0);
 	}
 
 	SequenceSet s;

scopehal/InstrumentChannel.cpp

@@ -151,3 +151,21 @@ bool InstrumentChannel::ShouldPersistWaveform()
 	//Default to persisting everything
 	return true;
 }
+
+////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+// Download progress
+
+InstrumentChannel::DownloadState InstrumentChannel::GetDownloadState()
+{
+	return DownloadState::DOWNLOAD_UNKNOWN;
+}
+
+float InstrumentChannel::GetDownloadProgress()
+{
+	return 0.0;
+}
+
+double InstrumentChannel::GetDownloadStartTime()
+{
+	return 0.0;
+}

scopehal/InstrumentChannel.h

@@ -239,6 +239,32 @@ class InstrumentChannel : public FlowGraphNode
 		VIS_SHOW,
 	} m_visibilityMode;
 
+	/**
+		@brief Enum values to be mapped to GetDownloadState() int result value for specific channel download states
+	 */
+	enum DownloadState
+	{
+		DOWNLOAD_UNKNOWN,
+		DOWNLOAD_NONE,					///< No download is pending (e.g. the scope is in stop mode)
+		DOWNLOAD_WAITING,				///< This channel is waiting to be downloaded (i.e. scope is triggered but previous channels are currently beeing downloaded)
+		DOWNLOAD_IN_PROGRESS,				///< Download has started
+		DOWNLOAD_FINISHED				///< Download is finished
+	};
+
+	/**
+		@brief Returns the current download state of this channel.
+
+		The returned int value can either be an integer ranging from 0 to 100 corresponding to the percentage of the waveform that has already been downloaded
+		or a (negative) int value to be mapped to the OscilloscopeChannel::DownloadState enum
+	 */
+	virtual DownloadState GetDownloadState();
+
+	/// returns the current completion of the download (on the range [0, 1]), if not DOWNLOAD_UNKNOWN
+	virtual float GetDownloadProgress();
+
+	/// returns the start time of a download, if we are DOWNLOAD_IN_PROGRESS; undefined, otherwise
+	virtual double GetDownloadStartTime();
+
 protected:
 
 	virtual void ClearStreams();

scopehal/Oscilloscope.cpp

@@ -856,74 +856,31 @@ bool Oscilloscope::IsInverted(size_t /*i*/)
 	return false;
 }
 
-// In automatic download progress bar display mode (i.e. if the driver doens not call SetShowChannelsDownloadProgress() method)
-// this is the maximum download duration under which the download progress bar will not be shown.
-#define SHOW_DOWNLOAD_PROGRESS_BAR_DURATION	0.3
-
-int Oscilloscope::GetChannelDownloadState(size_t i)
-{	// Surface check of channel index
-	if(i >= m_channelDownloadStates.size())
-		return OscilloscopeChannel::DownloadState::DOWNLOAD_NONE;
-
-	int result = m_channelDownloadStates[i];
-	if(!m_forceShowChannelProgressBar && !m_downloadSpeedEvaluated && 
-		result > OscilloscopeChannel::DownloadState::DOWNLOAD_STARTED && result < OscilloscopeChannel::DownloadState::DOWNLOAD_FINISHED)
-	{	// Check duration since start
-		double now = GetTime();
-		if((now - m_downloadStartTime) > SHOW_DOWNLOAD_PROGRESS_BAR_DURATION)
-		{	// We've been downloading for more than 300ms and are not finished yet => show progress bar
-			m_showChannelProgressBar = true;
-			m_downloadSpeedEvaluated = true;
-			//LogWarning("Download speed evaluation done, show progress = true : start time = %f, now = %f, diff = %f, state = %d)\n",m_downloadStartTime, now,now - m_downloadStartTime,result);
-		}
-	}
-	if(!m_showChannelProgressBar)
-		return OscilloscopeChannel::DownloadState::DOWNLOAD_PROGRESS_DISABLED;
-	return result;
-}
-
-void Oscilloscope::SetShowChannelsDownloadProgress(bool show)
-{	// Let the driver decide whether the progress bar should be shown or not
-	m_forceShowChannelProgressBar = true;
-	m_showChannelProgressBar = show;
-}
-
 void Oscilloscope::ChannelsDownloadStarted()
-{	// Set all channels to DOWNLOAD_WAITING state
-	m_channelDownloadStates.assign(m_channels.size(),OscilloscopeChannel::DownloadState::DOWNLOAD_WAITING);
-	if(!m_forceShowChannelProgressBar)
-	{	// Not in force mode => check if the sample depth has changed since last download
-		uint64_t newSampleDepth = GetSampleDepth();
-		size_t newEnabledChannels = GetEnabledChannelCount();
-		if((newSampleDepth != m_downloadSpeedEvalSampleDepth) || (newEnabledChannels != m_downloadSpeedEvalEnabledChannels))
-		{	// Sample depth has changed => re-evaluate download speed
-			m_downloadSpeedEvalSampleDepth = newSampleDepth;
-			m_downloadSpeedEvalEnabledChannels = newEnabledChannels;
-			m_downloadSpeedEvaluated = false;
-		}
-		else if(!m_downloadSpeedEvaluated)
-		{	// See how log elapsed since last download start
-			double now = GetTime();
-			if((now-m_downloadStartTime)<=SHOW_DOWNLOAD_PROGRESS_BAR_DURATION)
-			{	// Download fast enough, no need for a progress bar to be shown
-				m_downloadSpeedEvaluated = true;
-				m_showChannelProgressBar = false;
-				//LogWarning("Download speed evaluation done, show = false : start time = %f, now = %f, diff = %f)\n",m_downloadStartTime, now,now - m_downloadStartTime);
-			}
-		}
+{
+	for (size_t i = 0; i < m_channels.size(); i++)
+	{
+		auto chan = GetOscilloscopeChannel(i);
+		if (chan == nullptr)
+			continue;
+
+		chan->m_downloadState = InstrumentChannel::DownloadState::DOWNLOAD_WAITING;
+		chan->m_downloadProgress = 0.0;
+		chan->m_downloadStartTime = GetTime();
 	}
-	m_downloadStartTime = GetTime();
 }
 
-void Oscilloscope::UpdateChannelDownloadState(size_t i, int downloadState)
+void Oscilloscope::ChannelsDownloadStatusUpdate(size_t ch, InstrumentChannel::DownloadState state, float progress)
 {
-	if(i >= m_channelDownloadStates.size())
-	{	
-		m_channelDownloadStates.resize(i+1,OscilloscopeChannel::DownloadState::DOWNLOAD_NONE);
-	}
-	m_channelDownloadStates[i]=downloadState;
+	auto chan = GetOscilloscopeChannel(ch);
+	if (chan == nullptr)
+		return;
+
+	chan->m_downloadState = state;
+	chan->m_downloadProgress = progress;
 }
 
+
 ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 // Trigger configuration
 

scopehal/Oscilloscope.h

@@ -384,63 +384,13 @@ class Oscilloscope	: public virtual Instrument
 	 */
 	virtual bool IsInverted(size_t i);
 
-	/**
-		@brief Gets the download state of the specified channel.
-
-		The returned int value can either be an integer ranging from 0 to 100 corresponding to the percentage of the waveform that has already been downloaded,
-		or a (negative) int value to be mapped to the OscilloscopeChannel::DownloadState enum
-
-		@param i Zero-based index of channel
-	 */
-	virtual int GetChannelDownloadState(size_t i);
-
 protected:
 
-	/**
-		@brief Let the driver decide wehther channels download progress should be displayed or not, according to the current sample depth configuration.
-
-		If scope download speed is fast enough according the the currently set sample depth, progress should not be shown to prevent flickering effect on the download bar.
-		
-		If this method is not called by the driver, Oscilloscope class will try and determine whether download progress bar should be shown or not automatically.
-
-		@param show true if the download progress bar should be displayed, false otherwise
-	 */
-	void SetShowChannelsDownloadProgress(bool show);
-
-	/**
-		@brief Drivers should call this method at a download operation start to tell the Oscilloscope class to initialize all channel download states.
-
-	 */
+	/// @brief Helper method called by drivers to reset all channels to "waiting to download" state.
 	void ChannelsDownloadStarted();
 
-	/**
-		@brief Updates the download state for the specified channel.
-
-		The provided int value can either be an integer ranging from 0 to 100 corresponding to the percentage of the waveform that has already been downloaded
-		or a (negative) int value to be mapped to the OscilloscopeChannel::DownloadState enum
-
-		@param i Zero-based index of channel
-		@param downloadState the download state value
-
-	 */
-	void UpdateChannelDownloadState(size_t i, int downloadState);
-
-// Handling of waveform downnload operation state
-private:
-	// True when the download progress bar should be displayed (i.e. slow download speed)
-	bool m_showChannelProgressBar = false;
-	// True when the driver is handling download progressbar display state on its side
-	bool m_forceShowChannelProgressBar = false;
-	// Time of start of the latest download operation for this instrument
-	double m_downloadStartTime = 0;
-	// Sample depth that was set on this Oscilloscope the last time the download speed have been evaluated
-	uint64_t m_downloadSpeedEvalSampleDepth = 0;
-	// Number of enabled channels on this Oscilloscope the last time the download speed have been evaluated
-	size_t m_downloadSpeedEvalEnabledChannels;
-	// True whent the download speed has been evaluated
-	bool m_downloadSpeedEvaluated = false;
-	// Dowbload state for each channel of this Oscilloscope
-	std::vector<int> m_channelDownloadStates;
+	/// @brief Helper method called by drivers to set one channel's download status.
+	void ChannelsDownloadStatusUpdate(size_t ch, InstrumentChannel::DownloadState state, float progress);
 
 public:
 	//Triggering

scopehal/OscilloscopeChannel.cpp

@@ -61,6 +61,9 @@ OscilloscopeChannel::OscilloscopeChannel(
 	Stream::StreamType stype,
 	size_t index)
 	: InstrumentChannel(scope, hwname, color, xunit, yunit, stype, index)
+	, m_downloadState(DownloadState::DOWNLOAD_UNKNOWN)
+	, m_downloadProgress(0.0)
+	, m_downloadStartTime(0.0)
 	, m_refcount(0)
 {
 }
@@ -342,9 +345,17 @@ void OscilloscopeChannel::SetInputMux(size_t select)
 		GetScope()->SetInputMux(m_index, select);
 }
 
-int OscilloscopeChannel::GetDownloadState()
+InstrumentChannel::DownloadState OscilloscopeChannel::GetDownloadState()
 {
-	if(m_instrument)
-		return GetScope()->GetChannelDownloadState(m_index);
-	return DownloadState::DOWNLOAD_PROGRESS_DISABLED;
+	return m_downloadState;
+}
+
+float OscilloscopeChannel::GetDownloadProgress()
+{
+	return m_downloadProgress;
+}
+
+double OscilloscopeChannel::GetDownloadStartTime()
+{
+	return m_downloadStartTime;
 }

scopehal/OscilloscopeChannel.h

@@ -146,26 +146,15 @@ class OscilloscopeChannel : public InstrumentChannel
 
 	void SetDefaultDisplayName();
 
-
-	/**
-		@brief Enum values to be mapped to GetDownloadState() int result value for specific channel download states
-	 */
-	enum DownloadState : int 
-	{
-		DOWNLOAD_PROGRESS_DISABLED = -3, 	// Tell the UI not to show a download progress bar (e.g. if downloading the whole waveform is fast enough)
-		DOWNLOAD_NONE = -2,					// No download is pending (e.g. the scope is in stop mode)
-		DOWNLOAD_WAITING = -1,				// This channel is waiting to be downloaded (i.e. scope is triggered but previous channels are currently beeing downloaded)
-		DOWNLOAD_STARTED = 0,				// Download as tarted
-		DOWNLOAD_FINISHED = 100				// Download is finished
-	};
-
-	/**
-		@brief Returns the current download state of this channel.
-
-		The returned int value can either be an integer ranging from 0 to 100 corresponding to the percentage of the waveform that has already been downloaded
-		or a (negative) int value to be mapped to the OscilloscopeChannel::DownloadState enum
-	 */
-	int GetDownloadState();
+	virtual DownloadState GetDownloadState() override;
+	virtual float GetDownloadProgress() override;
+	virtual double GetDownloadStartTime() override;
+
+private:
+	// to be accessed by Oscilloscope to update download status
+	DownloadState m_downloadState;
+	float m_downloadProgress;
+	double m_downloadStartTime;
 
 protected:
 	void SharedCtorInit(Unit unit);

scopehal/RigolOscilloscope.cpp

@@ -858,10 +858,7 @@ bool RigolOscilloscope::AcquireData()
 		cap->m_startTimestamp = floor(now);
 		cap->m_startFemtoseconds = (now - floor(now)) * FS_PER_SECOND;
 
-		// Determine the size of one percent of the data to be downloaded
-		int percent = (m_highDefinition ? 2* npoints : npoints)/100;
-		int percentage = 0;
-		int restPercentage = 0;
+		ChannelsDownloadStatusUpdate(i, InstrumentChannel::DownloadState::DOWNLOAD_IN_PROGRESS, 0.0);
 
 		//Downloading the waveform is a pain in the butt, because we can only pull 250K points at a time! (Unless you have a MSO5)
 		for(size_t npoint = 0; npoint < npoints;)
@@ -942,28 +939,14 @@ bool RigolOscilloscope::AcquireData()
 			//Read actual block content and decode it
 			//Scale: (value - Yorigin - Yref) * Yinc
 
-			// Read it in slice so that we can smoothly update download progress bar
 			size_t bytesToRead = header_blocksize_bytes + 1; //trailing newline after data block
-			int slices = bytesToRead / percent;
-			int sliceRest = bytesToRead % percent;
-			size_t bytesRead = 0;
-			for(int s = 0 ; s < slices ; s++)
-			{	// Read each block corresponding to one percent of the total waveform
-				bytesRead += m_transport->ReadRawData(percent, (temp_buf+bytesRead));
-				percentage++;
-				UpdateChannelDownloadState(i,percentage);
-			}
-			if(bytesRead < bytesToRead)
-			{	// For the rest (bytesToRead % slices)
-				m_transport->ReadRawData(bytesToRead-bytesRead, (temp_buf+bytesRead));
-				restPercentage += sliceRest;
-				if(restPercentage>=percent && percentage < OscilloscopeChannel::DownloadState::DOWNLOAD_FINISHED)
-				{	// Accumulate leftover bytes untill we get a whole percent
-					percentage++;
-					UpdateChannelDownloadState(i,percentage);
-					restPercentage = 0;
-				}
-			}
+			auto downloadCallback = [i, this, npoint, npoints, bytesToRead] (float progress) {
+				/* we get the percentage of this particular download; convert this into linear percentage across all chunks */
+				float bytes_progress = npoint * (m_highDefinition ? 2 : 1) + progress * bytesToRead;
+				float bytes_total = npoints * (m_highDefinition ? 2 : 1);
+				ChannelsDownloadStatusUpdate(i, InstrumentChannel::DownloadState::DOWNLOAD_IN_PROGRESS, bytes_progress / bytes_total);
+			};
+			m_transport->ReadRawData(bytesToRead, temp_buf, downloadCallback);
 
 			double ydelta = yorigin + yreference;
 			cap->Resize(cap->m_samples.size() + header_blocksize);
@@ -993,7 +976,7 @@ bool RigolOscilloscope::AcquireData()
 			npoint += header_blocksize;
 		}
 		// Notify about end of download for this channel
-		UpdateChannelDownloadState(i,OscilloscopeChannel::DownloadState::DOWNLOAD_FINISHED);
+		ChannelsDownloadStatusUpdate(i, InstrumentChannel::DownloadState::DOWNLOAD_FINISHED, 1.0);
 
 		//Done, update the data
 		if(cap)

scopehal/SCPILinuxGPIBTransport.cpp

@@ -146,7 +146,7 @@ void SCPILinuxGPIBTransport::SendRawData(size_t len, const unsigned char* buf)
 	ibwrt(m_handle, (const char *)buf, len);
 }
 
-size_t SCPILinuxGPIBTransport::ReadRawData(size_t len, unsigned char* buf)
+size_t SCPILinuxGPIBTransport::ReadRawData(size_t len, unsigned char* buf, std::function<void(float)> /*progress*/)
 {
 	if (!IsConnected())
 		return 0;

scopehal/SCPILinuxGPIBTransport.h

@@ -56,7 +56,7 @@ class SCPILinuxGPIBTransport : public SCPITransport
 	void FlushRXBuffer(void) override;
 	bool SendCommand(const std::string& cmd) override;
 	std::string ReadReply(bool endOnSemicolon = true) override;
-	size_t ReadRawData(size_t len, unsigned char* buf) override;
+	size_t ReadRawData(size_t len, unsigned char* buf, std::function<void(float)> progress = nullptr) override;
 	void SendRawData(size_t len, const unsigned char* buf) override;
 
 	bool IsCommandBatchingSupported() override;