From a6daeef3f0f14b53fc87feb99c51ca25cd624a3c Mon Sep 17 00:00:00 2001
From: MALFREYT <alexandre.malfreyt@synchrotron-soleil.fr>
Date: Thu, 20 Feb 2025 17:39:46 +0100
Subject: [PATCH] refactor: improve code readability and structure in
 SingleShotAOManager

---
 src/SingleShotAOManager.cpp | 247 ++++++++++++++++++------------------
 1 file changed, 123 insertions(+), 124 deletions(-)

diff --git a/src/SingleShotAOManager.cpp b/src/SingleShotAOManager.cpp
index 6e6c2e7..7e70818 100755
--- a/src/SingleShotAOManager.cpp
+++ b/src/SingleShotAOManager.cpp
@@ -145,12 +145,12 @@ void SingleShotAOManager::init(asl::SingleShotAO * p_ssao, unsigned short p_nb_c
 		enable_periodic_msg(true);
 	}
 
-  // initialize channel indexes (-1 means no ramp in progress)
-  // and ramp states
-	for (unsigned int l_cpt = 0;l_cpt < m_nb_chan;l_cpt++)
+	// initialize channel indexes (-1 means no ramp in progress)
+	// and ramp states
+	for (unsigned int l_cpt = 0; l_cpt < m_nb_chan; l_cpt++)
 	{
 		m_currentIndex[l_cpt] = -1;
-    m_isRunning[l_cpt] = false;
+    	m_isRunning[l_cpt] = false;
 	}
 }
 
@@ -164,36 +164,39 @@ void SingleShotAOManager::process_message (yat::Message& msg)
 	switch (msg.type())
 	{
 		//- THREAD_INIT ----------------------
-	case yat::TASK_INIT:
+		case yat::TASK_INIT:
 		{
 			DEBUG_STREAM << "SingleShotAOManager::handle_message::THREAD_INIT::thread is starting up" << std::endl;
 		} 
 		break;
 
 		//- THREAD_EXIT ----------------------
-	case yat::TASK_EXIT:
+		case yat::TASK_EXIT:
 		{
 			DEBUG_STREAM << "SingleShotAOManager::handle_message::THREAD_EXIT::thread is quitting" << std::endl;
 		}
 		break;
 
 		//- THREAD_PERIODIC ------------------
-	case yat::TASK_PERIODIC:
+		case yat::TASK_PERIODIC:
 		{
 			periodic_job_i();
 		}
 		break;
 
 		//- THREAD_TIMEOUT -------------------
-	case yat::TASK_TIMEOUT:
+		case yat::TASK_TIMEOUT:
 		{
 			//- not used in this example
 		}
 		break;
+
 		//- UNHANDLED MSG --------------------
-	default:
-		DEBUG_STREAM << "SingleShotAOManager::handle_message::unhandled msg type received" << std::endl;
-		break;
+		default:
+		{
+			DEBUG_STREAM << "SingleShotAOManager::handle_message::unhandled msg type received" << std::endl;
+			break;
+		}
 	}
 }
 
@@ -202,53 +205,52 @@ void SingleShotAOManager::process_message (yat::Message& msg)
 // ============================================================================ 
 void SingleShotAOManager::periodic_job_i()
 {
-	//test all channels
+	// test all channels
 	for (unsigned int l_cpt = 0;l_cpt < m_nb_chan;l_cpt++)
 	{
-		//test if a ramp step must occur
-		if (m_currentIndex[l_cpt] != -1)
+		// test if a ramp step must occur
+		if (m_currentIndex[l_cpt] == -1)
 		{
-			m_isRunning[l_cpt] = true;
-			double l_val = 0;
-			l_val = m_ramps[l_cpt][m_currentIndex[l_cpt]];
-			DEBUG_STREAM << "Current value for channel" << l_cpt << ": " << l_val << endl;
-			try 
-			{
-				CHECK_SSAO();
-				m_ssao->write_scaled_channel((adl::ChanId)l_cpt, l_val);
-				m_channels[l_cpt] = l_val;
-			}
-			catch(const asl::DAQException& de)
-			{
-				Tango::DevFailed df = daq_to_tango_exception(de);
-				ERROR_STREAM << df<< endl;
-				m_state = Tango::FAULT;
-				RETHROW_DEVFAILED(df,
-					"DRIVER_FAILURE",
-					"could not write channel [caught asl::DAQException]",
-					"SingleShotAOManager::write_channel");
-			}
-			catch(...)
-			{
-				ERROR_STREAM << "SingleShotAOManager::write_channel::unknown exception caught"<<std::endl;
-				m_state = Tango::FAULT;
-				THROW_DEVFAILED("DRIVER_FAILURE",
-					"could not write channel [unknown error]",
-					"SingleShotAOManager::write_channel");
-			}
-			
-			//check if there is another value
-			m_currentIndex[l_cpt] +=1;
-			if (m_currentIndex[l_cpt] == m_ramps[l_cpt].capacity())
-			{
-				m_currentIndex[l_cpt] = -1;
-				m_initials[l_cpt] = m_channels[l_cpt];
-				m_ramps[l_cpt].clear();
-			}
+			m_isRunning[l_cpt] = false;
+			continue;
 		}
-		else
+
+		m_isRunning[l_cpt] = true;
+		double l_val = 0;
+		l_val = m_ramps[l_cpt][m_currentIndex[l_cpt]];
+		DEBUG_STREAM << "Current value for channel" << l_cpt << ": " << l_val << endl;
+		try 
 		{
-			m_isRunning[l_cpt] = false;
+			CHECK_SSAO();
+			m_ssao->write_scaled_channel((adl::ChanId)l_cpt, l_val);
+			m_channels[l_cpt] = l_val;
+		}
+		catch(const asl::DAQException& de)
+		{
+			Tango::DevFailed df = daq_to_tango_exception(de);
+			ERROR_STREAM << df << endl;
+			m_state = Tango::FAULT;
+			RETHROW_DEVFAILED(df,
+				"DRIVER_FAILURE",
+				"could not write channel [caught asl::DAQException]",
+				"SingleShotAOManager::write_channel");
+		}
+		catch(...)
+		{
+			ERROR_STREAM << "SingleShotAOManager::write_channel::unknown exception caught" << std::endl;
+			m_state = Tango::FAULT;
+			THROW_DEVFAILED("DRIVER_FAILURE",
+				"could not write channel [unknown error]",
+				"SingleShotAOManager::write_channel");
+		}
+		
+		// check if there is another value
+		m_currentIndex[l_cpt] += 1;
+		if (m_currentIndex[l_cpt] == m_ramps[l_cpt].capacity())
+		{
+			m_currentIndex[l_cpt] = -1;
+			m_initials[l_cpt] = m_channels[l_cpt];
+			m_ramps[l_cpt].clear();
 		}
 	}
 }
@@ -266,8 +268,9 @@ double SingleShotAOManager::get_channel(ChannelId_t p_chIdx)
 // ============================================================================ 
 void SingleShotAOManager::write_channel(ChannelId_t p_chIdx, double p_val)
 {
-	DEBUG_STREAM << "write_channel : " << p_chIdx << " : " << p_val << " : " << endl;
+	DEBUG_STREAM << "write_channel " << p_chIdx << " : " << p_val << endl;
 
+	// if the speed is 0, write the value directly
 	if (m_speeds[p_chIdx] == 0.0)
 	{
 		try 
@@ -276,11 +279,12 @@ void SingleShotAOManager::write_channel(ChannelId_t p_chIdx, double p_val)
 			m_ssao->write_scaled_channel((adl::ChanId)p_chIdx, p_val);
 			m_channels[p_chIdx] = p_val;
 			m_initials[p_chIdx] = p_val;
+			DEBUG_STREAM << "Speed is 0, writing directly the value" << endl;
 		}
 		catch(const asl::DAQException& de)
 		{
 			Tango::DevFailed df = daq_to_tango_exception(de);
-			ERROR_STREAM << df<< endl;
+			ERROR_STREAM << df << endl;
 			m_state = Tango::FAULT;
 			RETHROW_DEVFAILED(df,
 				"DRIVER_FAILURE",
@@ -295,83 +299,78 @@ void SingleShotAOManager::write_channel(ChannelId_t p_chIdx, double p_val)
 				"could not write channel [unknown error]",
 				"SingleShotAOManager::write_channel");
 		}
+		return;
 	}
-	else
+
+	// if a ramp is not running, error
+	if (m_isRunning[p_chIdx])
+	{
+		THROW_DEVFAILED("DEVICE_FAILURE",
+			"could not write channel : a ramp is still in progress on this channel",
+			"SingleShotAOManager::write_channel");
+	}
+
+	// if frequency = 0, error
+	if (m_frequency == 0)
+	{
+		THROW_DEVFAILED("DRIVER_FAILURE",
+			"could not set a ramp on this channel. The frequency is 0",
+			"SingleShotAOManager::write_channel");
+	}
+
+	// if initial = channel, skip
+	if (m_initials[p_chIdx] == p_val)
+	{
+		DEBUG_STREAM << "Initial value is the same as the given value, skipping" << endl;
+		return;
+	}
+
+	// ramp determination
+	double l_delta = p_val - m_initials[p_chIdx];
+	bool isDown = false;
+	l_delta  = ((l_delta * m_frequency) / m_speeds[p_chIdx]) + 1;
+	if (l_delta < 0)
+	{
+		l_delta = -l_delta + 2;
+		isDown = true;
+	}
+	DEBUG_STREAM << "Computed ramp steps number : " << l_delta << endl;
+
+	yat::Buffer<double> l_buffer;
+	size_t ramp_size = (size_t)(ceil(l_delta));
+	l_buffer.capacity(ramp_size);
+	l_buffer.force_length(ramp_size);
+	
+	// check if ramp step is integer or not
+	bool isDeltaNotInt = (ramp_size != ((size_t)(floor(l_delta))));
+	DEBUG_STREAM << "Real ramp steps number : " << ramp_size << endl;
+
+	for (unsigned int l_cpt = 0; l_cpt < ramp_size; l_cpt++)
 	{
-		// check if a ramp is not running
-		if (!m_isRunning[p_chIdx])
+		if ((l_cpt == (ramp_size - 1)) && (isDeltaNotInt))
 		{
-			// check if initial = channel
-			if (m_initials[p_chIdx] != p_val)
-			{
-				if (m_frequency == 0)
-				{
-					THROW_DEVFAILED("DRIVER_FAILURE",
-						"could not set a ramp on this channel. The frequency is 0",
-						"SingleShotAOManager::write_channel");
-				}
-				
-				//ramp determination
-				double l_delta = p_val - m_initials[p_chIdx];
-				bool isDown = false;
-				l_delta  = ((l_delta * m_frequency) / m_speeds[p_chIdx])+1;
-				if (l_delta < 0)
-				{
-					l_delta = -l_delta + 2;
-					isDown = true;
-				}
-				DEBUG_STREAM << "Computed ramp steps number : " << l_delta << endl;
-								
-				yat::Buffer<double> l_buffer;
-        size_t ramp_size = (size_t)(ceil(l_delta));
-				l_buffer.capacity(ramp_size);
-				l_buffer.force_length(ramp_size);
-				
-				// check if ramp step is integer or not
-				bool isDeltaNotInt = false;
-				if (ramp_size != ((size_t)(floor(l_delta))))
-				{
-				  isDeltaNotInt = true;
-				}
-
-        DEBUG_STREAM << "Real ramp steps number : " << ramp_size << endl;
-
-				for (unsigned int l_cpt = 0; l_cpt < ramp_size; l_cpt++)
-				{
-				  if ((l_cpt == (ramp_size - 1)) && 
-              (isDeltaNotInt))
-				  {
-					  // add the setpoint value at the end of table
-					  l_buffer[l_cpt] = p_val;
-				  }
-				  else
-				  {
-					  if (isDown)
-					  {
-						  l_buffer[l_cpt] = m_initials[p_chIdx] - l_cpt*(m_speeds[p_chIdx]/m_frequency);
-					  }
-					  else
-					  {
-						  l_buffer[l_cpt] = m_initials[p_chIdx] + l_cpt*(m_speeds[p_chIdx]/m_frequency);
-					  }
-				  }
-          //DEBUG_STREAM << "Ramp buffer[" << l_cpt << "] = " << l_buffer[l_cpt] << endl;
-				}
-				m_ramps[p_chIdx].clear();
-				m_ramps[p_chIdx].capacity(0);
-				m_ramps[p_chIdx].force_length(0);
-				m_currentIndex[p_chIdx] = 0;
-				m_ramps[p_chIdx] = l_buffer;
-				//m_channels[p_chIdx] = m_ramps[p_chIdx][0]; -- soso on ne met rien ici => à l'application
-			}
+			// add the setpoint value at the end of table
+			l_buffer[l_cpt] = p_val;
 		}
 		else
 		{
-			THROW_DEVFAILED("DEVICE_FAILURE",
-				"could not write channel : a ramp is still in progress on this channel",
-				"SingleShotAOManager::write_channel");
+			if (isDown)
+			{
+				l_buffer[l_cpt] = m_initials[p_chIdx] - l_cpt * (m_speeds[p_chIdx] / m_frequency);
+			}
+			else
+			{
+				l_buffer[l_cpt] = m_initials[p_chIdx] + l_cpt * (m_speeds[p_chIdx] / m_frequency);
+			}
 		}
+		//DEBUG_STREAM << "Ramp buffer[" << l_cpt << "] = " << l_buffer[l_cpt] << endl;
 	}
+	m_ramps[p_chIdx].clear();
+	m_ramps[p_chIdx].capacity(0);
+	m_ramps[p_chIdx].force_length(0);
+	m_currentIndex[p_chIdx] = 0;
+	m_ramps[p_chIdx] = l_buffer;
+	//m_channels[p_chIdx] = m_ramps[p_chIdx][0]; -- soso on ne met rien ici => à l'application
 }
 
 // ============================================================================
-- 
GitLab