Implemented Delete

Delete works like this. FileMonitor scans for new, changed or deleted
files. If it sees any changes it updates the page list. Delete is
especially complicated as we want to avoid threads from crashing each
other, by deleting an object that the Service thread is still using.
Ideally needs a mutex but with careful timing it should work OK. At
least most of the time.

filemonitor.cpp

@@ -72,12 +72,12 @@ void FileMonitor::run()
       std::cerr << "Error(" << errno << ") opening " << path << std::endl;
       return;
     }
-		
+
 		_pageList->ClearFlags(); // Assume that no files exist
 
     // Load the filenames into a list
     while ((dirp = readdir(dp)) != NULL)
-    {			
+    {
       // Select only pages that might be teletext. tti or ttix at the moment.
 			// strcasestr doesn't seem to be in my Windows compiler.
 #ifdef _WIN32
@@ -104,7 +104,7 @@ void FileMonitor::run()
         if (p) // File was found
         {
           //std::cerr << dirp->d_name << " was found" << std::endl;
-          
+
           if (attrib.st_mtime!=p->GetModifiedTime()) // File exists. Has it changed?
           {
             std::cerr << "File has been modified" << dirp->d_name << std::endl;
@@ -116,42 +116,35 @@ void FileMonitor::run()
             p->SetModifiedTime(attrib.st_mtime);
             // unlock
 
-            //Load in the modified file. That should also reset the file time.
-            // 1) Lock the page. We can't transmit it
-            // 2) Remove it from PageList
-            // 3) Delete the page.
-            // 4) Load the updated page
-            // 5) Add it to PageList
-            // 6) Remove the lock
           }
-					p->SetExistsFlag(); // Mark this page as existing on the drive
+					p->SetState(TTXPageStream::FOUND); // Mark this page as existing on the drive
         }
         else
         {
           std::cerr << "[FileMonitor::run] " << " Adding a new page" << dirp->d_name << std::endl;
 					// A new file. Create the page object and add it to the page list.
 					if ((p=new TTXPageStream(name)))
 					{
-						//p->SetModifiedTime(attrib.st_mtime); // This line is redundant
 						_pageList->AddPage(p);
 					}
 					else
 						std::cerr << "[FileMonitor::run] Failed to load" << dirp->d_name << std::endl;
         }
-
-        // 2) Is it an existing page that has changed?
-        // 3) Are there any pages that no longer exist?
       }
     }
     closedir(dp);
-    std::cerr << "FINISHED LOADING PAGES" << std::endl;
-		
-		// Delete pages that no longer exist
+    std::cerr << "[FileMonitor::run] Finished scan" << std::endl;
+
+		// Delete pages that no longer exist (this blocks the thread until the pages are removed)
 		_pageList->DeleteOldPages();
 
-    // Wait for ms/1000 seconds
+    // Wait 5 seconds.
+    // WARNING. We must allow enough time for Service to complete the delete or this process might crash
+    // Sounds like a job for a mutex.
     struct timespec rec;
-    int ms=5000;
+    int ms;
+
+    ms=5000;
     rec.tv_sec = ms / 1000;
     rec.tv_nsec=(ms % 1000) *1000000;
     nanosleep(&rec,NULL);

mag.cpp

@@ -78,7 +78,7 @@ Packet* Mag::GetPacket(Packet* p)
     // If there is no page, we should send a filler
     if (_pageSet->size()<1)
     {
-      _outp("V"); // If this goes wrong we are getting _pageSet wrong
+      //_outp("V"); // If this goes wrong we are getting _pageSet wrong
         return filler;
     }
 
@@ -107,26 +107,42 @@ Packet* Mag::GetPacket(Packet* p)
 		switch (_state)
 		{
 		case STATE_HEADER: // Decide which page goes next
-          _outp("h");
+          //_outp("h");
 				_headerFlag=true;
         _page=GetCarouselPage(); // Is there a carousel page due?
 
+        // Is this page deleted?
+        if (_page && _page->GetStatusFlag()==TTXPageStream::MARKED)
+        {
+          std::cerr << "[Mag::GetPacket] Delete this carousel" << std::endl;
+          _carousel->deletePage(_page);
+          _page=NULL;
+          // @todo We are not done. This just deletes a pointer to the page. It is still in _pageList
+        }
+
 
         if (_page) // Carousel? Step to the next subpage
 				{
-          _outp("c");
+          //_outp("c");
 					_page->StepNextSubpage();
 				}
 				else  // No carousel? Take the next page in the main sequence
         {
-          _outp("n");
+          //_outp("n");
             ++_it;
             if (_it==_pageSet->end())
             {
                 _it=_pageSet->begin();
             }
             // Get pointer to the page we are sending
             _page=&*_it;
+            if (_page->GetStatusFlag()==TTXPageStream::MARKED)
+            {
+              _pageSet->remove(*(_it++));
+              _page=NULL;
+              return filler;
+              // Stays in HEADER mode so that we run this again
+            }
         }
 				// @todo: This assert fails!
         // assert(_page->GetPageNumber()>>16 == _magNumber); // Make sure that we always point to the correct magazine
@@ -197,11 +213,11 @@ Packet* Mag::GetPacket(Packet* p)
 				p=NULL;
 				_state=STATE_HEADER;
 				_thisRow=0;
-				_outp("H");
+				//_outp("H");
 			}
 			else
 			{
-        _outp("J");
+        //_outp("J");
 				if (txt->GetLine().empty() && false) // If the row is empty then skip it (Kill this for now)
 				{
 					// std::cerr << "[Mag::GetPacket] Empty row" << std::hex << _page->GetPageNumber() << std::dec << std::endl;
@@ -220,7 +236,7 @@ Packet* Mag::GetPacket(Packet* p)
 
 			break;
 		} // switch
-    if (p==NULL) _outp("q");
+    //if (p==NULL) _outp("q");
     return p;
 }
 
@@ -229,6 +245,6 @@ void Mag::_outp(std::string s)
 {
   return;
   if (_magNumber==5)
-    std::cout << s;
+    std::cerr << s;
 }
 

mag.h

@@ -44,6 +44,7 @@ class Mag
         std::list<TTXPageStream>*  Get_pageSet() { return _pageSet; }
         /** GetPacket
          * \return the next packet in the magazine stream or NULL if this was the last packet
+         * @param p is a packet instance that will be updated
          */
         Packet* GetPacket(Packet* p);
 

pagelist.cpp

@@ -38,17 +38,19 @@ int PageList::LoadPageList(std::string filepath)
 	while ((dirp = readdir(dp)) != NULL)
 	{
 		//p=new TTXPageStream(filepath+"/"+dirp->d_name);
-
-		q=new TTXPageStream(filepath+"/"+dirp->d_name);
-		// If the page loaded, then push it into the appropriate magazine
-		if (q->Loaded())
+    if (std::string(dirp->d_name).find(".tti") != std::string::npos)	// Is the file type .tti or ttix?
     {
-        int mag=(q->GetPageNumber() >> 16) & 0x7;
-        _pageList[mag].push_back(*q);
+      q=new TTXPageStream(filepath+"/"+dirp->d_name);
+      // If the page loaded, then push it into the appropriate magazine
+      if (q->Loaded())
+      {
+          int mag=(q->GetPageNumber() >> 16) & 0x7;
+          _pageList[mag].push_back(*q); // This copies. But we can't copy a mutex
+      }
     }
 	}
 	closedir(dp);
-  std::cerr << "FINISHED LOADING PAGES" << std::endl;
+  std::cerr << "[PageList::LoadPageList]FINISHED LOADING PAGES" << std::endl;
 
 	// How many files did we accept?
 	for (int i=0;i<8;i++)
@@ -88,6 +90,7 @@ void PageList::AddPage(TTXPageStream* page)
 
 TTXPageStream* PageList::Locate(std::string filename)
 {
+  // This is called from the FileMonitor thread
   // std::cerr << "[PageList::Locate] *** TODO *** " << filename << std::endl;
   for (int mag=0;mag<8;mag++)
   {
@@ -102,7 +105,7 @@ TTXPageStream* PageList::Locate(std::string filename)
     }
 
   }
-  return NULL; // @todo placeholder
+ return NULL; // @todo placeholder What should we do here?
 }
 
 // Detect pages that have been deleted from the drive
@@ -117,23 +120,44 @@ void PageList::ClearFlags()
     {
       TTXPageStream* ptr;
       ptr=&(*p);
-			ptr->ClearExistsFlag();
+      // Don't unmark a file that was MARKED. Once condemned it won't be pardoned
+      if (ptr->GetStatusFlag()==TTXPageStream::FOUND)
+      {
+        ptr->SetState(TTXPageStream::NOTFOUND);
+      }
     }
   }
 }
 
 void PageList::DeleteOldPages()
 {
+  // This is called from the FileMonitor thread
   for (int mag=0;mag<8;mag++)
   {
     for (std::list<TTXPageStream>::iterator p=_pageList[mag].begin();p!=_pageList[mag].end();++p)
     {
       TTXPageStream* ptr;
       ptr=&(*p);
-			if (!ptr->GetExistsFlag())
-			{
-				std::cerr << "[PageList::DeleteOldPages] deleting " << ptr->GetSourcePage() << std::endl;
-			}
+      if (ptr->GetStatusFlag()==TTXPageStream::NOTFOUND)
+      {
+				std::cerr << "[PageList::DeleteOldPages] Marked for Delete " << ptr->GetSourcePage() << std::endl;
+				// Pages marked here get deleted in the Service thread
+        ptr->SetState(TTXPageStream::MARKED);
+      }
     }
   }
 }
+
+
+/* Want this to happen in the Service thread.
+      // Not the best idea, to check for deletes here
+      if (_fileToDelete==ptr->GetSourcePage()) // This works but isn't great. Probably not too efficient
+      {
+        std::cerr << "[PageList::Locate]ready to delete " << ptr->GetSourcePage() << std::endl;
+        _fileToDelete="null"; // Signal that delete has been done.
+        //@todo Delete the page object, remove it from pagelist, fixup mag, skip to the next page
+        _pageList[mag].remove(*(p++)); // Also post-increment the iterator OR WE CRASH!
+        // We can do this safely because this thread is in the Service thread and won't clash WRONG!
+      }
+
+*/

pagelist.h

@@ -14,7 +14,7 @@
 
 namespace ttx
 {
-/** A PageList maintains the set of all teletext pages in a teletext service
+/** @brief A PageList maintains the set of all teletext pages in a teletext service
  *  It can load, save, add, delete, edit pages.
  *  Internally each magazine has its own list of pages.
  *
@@ -43,14 +43,14 @@ class PageList
   /** Add a teletext page to the proper magazine
    * @param page TTXPageStream object that has already been loaded
    */
-	void AddPage(TTXPageStream* page);	
-	
-  /** Clear all the exists flags 
+	void AddPage(TTXPageStream* page);
+
+  /** Clear all the exists flags
    */
 	void ClearFlags();
   /** Delete all pages that no longer exist
    */
-	void DeleteOldPages();	
+	void DeleteOldPages();
 
 private:
 	Configure* _configure; // The configuration object

service.cpp

@@ -2,8 +2,6 @@
  */
 #include "service.h"
 
-#include <ctime>
-
 using namespace ttx;
 
 Service::Service()
@@ -117,7 +115,7 @@ int Service::run()
       bool isHeader=false;
       if (pkt!=NULL) // How could this be NULL? After the last packet of a page has been sent.
       {
-        isHeader=mag[nmag]->GetHeaderFlag(); // pkt->IsHeader();
+        isHeader=pMag->GetHeaderFlag(); // pkt->IsHeader();
         std::string s=pkt->tx();
         if (s.length()<42)
         {

service.h

@@ -4,6 +4,8 @@
 #include <iostream>
 #include <iomanip>
 #include <thread>
+#include <ctime>
+
 #include "configure.h"
 #include "pagelist.h"
 #include "packet.h"

ttxpage.h

@@ -246,9 +246,6 @@ class TTXPage
          */
         bool m_LoadTTX(std::string filename);
 
-
-
-
 };
 
 #endif // TTXPAGE_H

ttxpagestream.cpp

@@ -4,7 +4,7 @@ TTXPageStream::TTXPageStream() :
      _isCarousel(false),
      _transitionTime(0),
 		 _CarouselPage(this),
-		 _existsOnDrive(false)
+		 _fileStatus(NEW)
 {
     //ctor
 }
@@ -14,7 +14,7 @@ TTXPageStream::TTXPageStream(std::string filename) :
      _isCarousel(false),
      _transitionTime(0),
 		 _CarouselPage(this),
-		 _existsOnDrive(false)
+		 _fileStatus(NEW)
 {
   struct stat attrib;               // create a file attribute structure
   stat(filename.c_str(), &attrib);  // get the attributes of the file
@@ -73,3 +73,10 @@ bool TTXPageStream::LoadPage(std::string filename)
   return Loaded;
 }
 
+bool TTXPageStream::operator==(const TTXPageStream& rhs) const
+{
+  // std::cerr << "operator overloaded == " << rhs.GetSourcePage();
+  if (this->GetSourcePage()==rhs.GetSourcePage())
+    return true;
+  return false;
+}