From 974275de94cf2a2b10c6909cda064208d2280abc Mon Sep 17 00:00:00 2001 From: Peter Kwan Date: Mon, 21 Aug 2017 11:50:01 +0100 Subject: [PATCH] Sort of producing output now. Reworked the service loop to improve the packet sequencing. Replaced NULL with nullptr as that is the C+11 way to do things. --- carousel.cpp | 6 ++--- filemonitor.cpp | 4 ++-- mag.cpp | 16 ++++++------- packetmag.cpp | 60 ++++++++++++++++++++++++++++++++++--------------- packetmag.h | 3 +-- packetsource.h | 7 ++++-- pagelist.cpp | 4 ++-- service.cpp | 49 +++++++++++++++++++++++++++++++--------- ttxpagestream.h | 4 ++-- 9 files changed, 104 insertions(+), 49 deletions(-) diff --git a/carousel.cpp b/carousel.cpp index 9aaa3b1..0e49bd2 100644 --- a/carousel.cpp +++ b/carousel.cpp @@ -32,7 +32,7 @@ void Carousel::deletePage(TTXPageStream* p) TTXPageStream* Carousel::nextCarousel() { TTXPageStream* p; - //std::cerr << "[nextCarousel] list size = " << _carouselList.size() << std::endl; + std::cerr << "[nextCarousel] list size = " << _carouselList.size() << std::endl; if (_carouselList.size()==0) return NULL; @@ -42,11 +42,11 @@ TTXPageStream* Carousel::nextCarousel() if (p->Expired()) { - //std::cerr << "[Carousel::nextCarousel] page " << std::hex << p->GetPageNumber() << std::dec << " cycle time=" << p->GetCycleTime() << std::endl; + std::cerr << "[Carousel::nextCarousel] page " << std::hex << p->GetPageNumber() << std::dec << " cycle time=" << p->GetCycleTime() << std::endl; p->SetTransitionTime(); // We found a carousel that is ready to step break; } - p=NULL; + p=nullptr; } #if 0 char c; diff --git a/filemonitor.cpp b/filemonitor.cpp index f967d20..44d5423 100644 --- a/filemonitor.cpp +++ b/filemonitor.cpp @@ -34,7 +34,7 @@ FileMonitor::FileMonitor(Configure *configure, PageList *pageList) : } FileMonitor::FileMonitor() - : _pageList(NULL) + : _pageList(nullptr) { //ctor } @@ -146,6 +146,6 @@ void FileMonitor::run() ms=5000; rec.tv_sec = ms / 1000; rec.tv_nsec=(ms % 1000) *1000000; - nanosleep(&rec,NULL); + nanosleep(&rec,nullptr); } } // run diff --git a/mag.cpp b/mag.cpp index 9dab238..f19ef78 100644 --- a/mag.cpp +++ b/mag.cpp @@ -12,7 +12,7 @@ Mag::Mag(int mag, std::list* pageSet, ttx::Configure *configure) _headerFlag(false), _thisRow(0), _state(STATE_HEADER), - _lastTxt(NULL) + _lastTxt(0) { //ctor if (_pageSet->size()>0) @@ -111,7 +111,7 @@ Packet* Mag::GetPacket(Packet* p) { //std::cerr << "[Mag::GetPacket] Delete this carousel" << std::endl; _carousel->deletePage(_page); - _page=NULL; + _page=nullptr; // @todo We are not done. This just deletes a pointer to the page. It is still in _pageList } @@ -143,7 +143,7 @@ Packet* Mag::GetPacket(Packet* p) if (_page->GetStatusFlag()==TTXPageStream::MARKED) { _pageSet->remove(*(_it++)); - _page=NULL; + _page=nullptr; return filler; // Stays in HEADER mode so that we run this again } @@ -151,8 +151,8 @@ Packet* Mag::GetPacket(Packet* p) { // todo: // std::cerr << "Page is a carousel. This can not happen" << std::endl; - _page=NULL; - return NULL; + _page=nullptr; + return nullptr; } } @@ -283,7 +283,7 @@ Packet* Mag::GetPacket(Packet* p) _state=STATE_TEXTROW; // Fall through to text rows on normal pages } else { // otherwise we end the page here - p=NULL; + p=nullptr; _state=STATE_HEADER; _thisRow=0; break; @@ -301,7 +301,7 @@ Packet* Mag::GetPacket(Packet* p) { if(_page->GetPageCoding() == CODING_7BIT_TEXT){ // if this is a normal page we've finished - p=NULL; + p=nullptr; _state=STATE_HEADER; _thisRow=0; //_outp("H"); @@ -317,7 +317,7 @@ Packet* Mag::GetPacket(Packet* p) if (_lastTxt->IsBlank() && _configure->GetRowAdaptive()) // If the row is empty then skip it { // std::cerr << "[Mag::GetPacket] Empty row" << std::hex << _page->GetPageNumber() << std::dec << std::endl; - p=NULL; + p=nullptr; } else { diff --git a/packetmag.cpp b/packetmag.cpp index 63abb41..013a6c3 100644 --- a/packetmag.cpp +++ b/packetmag.cpp @@ -9,14 +9,14 @@ using namespace vbit; PacketMag::PacketMag(uint8_t mag, std::list* pageSet, ttx::Configure *configure, uint8_t priority) : _pageSet(pageSet), _configure(configure), - _page(NULL), + _page(nullptr), _magNumber(mag), _priority(priority), _priorityCount(priority), _headerFlag(false), _state(PACKETSTATE_HEADER), _thisRow(0), - _lastTxt(NULL) + _lastTxt(nullptr) { //ctor if (_pageSet->size()>0) @@ -38,6 +38,7 @@ PacketMag::~PacketMag() // @todo Invent a packet sequencer similar to mag.cpp which this will replace Packet* PacketMag::GetPacket(Packet* p) { + std::cerr << "[PacketMag::GetPacket] mag=" << _magNumber << " state=" << _state << std::endl; int thisPageNum; unsigned int thisSubcode; int thisStatus; @@ -46,11 +47,16 @@ Packet* PacketMag::GetPacket(Packet* p) static vbit::Packet* filler=new Packet(8,25," "); // filler // We should only call GetPacket if IsReady has returned true + + /* Nice to have a safety net + * but without the previous value of the force flag this can give a false positive. + if (!IsReady()) { std::cerr << "[PacketMag::GetPacket] Packet not ready. This must not happen" << std::endl; exit(0); } + */ // If there is no page, we should send a filler if (_pageSet->size()<1) @@ -63,7 +69,7 @@ Packet* PacketMag::GetPacket(Packet* p) case PACKETSTATE_HEADER: // Start to send out a new page, which may be a simple page or one of a carousel ClearEvent(EVENT_FIELD); // This will suspend all packets until the next field. - _page=_carousel->nextCarousel(); // The next carousel page + _page=_carousel->nextCarousel(); // The next carousel page (if there is one) // But before that, do some housekeeping @@ -71,7 +77,7 @@ Packet* PacketMag::GetPacket(Packet* p) if (_page && _page->GetStatusFlag()==TTXPageStream::MARKED) { _carousel->deletePage(_page); - _page=NULL; + _page=nullptr; } if (_page) // Carousel? Step to the next subpage @@ -85,7 +91,7 @@ Packet* PacketMag::GetPacket(Packet* p) { if (_it==_pageSet->end()) { - std::cerr << "This can not happen" << std::endl; + std::cerr << "This can not happen (we can't get the next page?)" << std::endl; exit(0); } ++_it; @@ -100,17 +106,17 @@ Packet* PacketMag::GetPacket(Packet* p) if (_page->GetStatusFlag()==TTXPageStream::MARKED) { _pageSet->remove(*(_it++)); - _page=NULL; + _page=nullptr; return filler; // Stays in HEADER mode so that we run this again } if (_page->IsCarousel() && _page->GetCarouselFlag()) // Don't let registered carousel pages into the main page sequence { - std::cerr << "This can not happen" << std::endl; - exit(0); + std::cerr << "This can not happen. Carousel found but it isn't a carousel?" << std::endl; + // exit(0); // @todo MUST FIX THIS. Need to find out how we are getting here and stop it doing that! // Page is a carousel. This can not happen - //_page=NULL; - //return NULL; + _page=nullptr; // clear everything for now so that we keep running @todo THIS IS AN ERROR + return nullptr; } } _thisRow=0; @@ -166,16 +172,20 @@ Packet* PacketMag::GetPacket(Packet* p) // Find the next row that isn't NULL for (_thisRow++;_thisRow<26;_thisRow++) { + std::cerr << "*"; _lastTxt=_page->GetTxRow(_thisRow); if (_lastTxt!=NULL) break; } + std::cerr << std::endl; + std::cerr << "[PacketMag::GetPacket] TEXT ROW sending row" << _thisRow << std::endl; + // Didn't find? End of this page. if (_thisRow>25 || _lastTxt==NULL) { if(_page->GetPageCoding() == CODING_7BIT_TEXT){ // if this is a normal page we've finished - p=NULL; + p=nullptr; _state=PACKETSTATE_HEADER; _thisRow=0; //_outp("H"); @@ -191,7 +201,7 @@ Packet* PacketMag::GetPacket(Packet* p) if (_lastTxt->IsBlank() && _configure->GetRowAdaptive()) // If the row is empty then skip it { // std::cerr << "[Mag::GetPacket] Empty row" << std::hex << _page->GetPageNumber() << std::dec << std::endl; - p=NULL; + p=nullptr; } else { @@ -204,11 +214,13 @@ Packet* PacketMag::GetPacket(Packet* p) } break; case PACKETSTATE_FASTEXT: +// std::cerr << "PACKETSTATE_FASTEXT enters" << std::endl; p->SetMRAG(_magNumber,27); links=_page->GetLinkSet(); p->Fastext(links,_magNumber); _lastTxt=_page->GetTxRow(28); // Get _lastTxt ready for packet 28 processing _state=PACKETSTATE_PACKET28; +// std::cerr << "PACKETSTATE_FASTEXT exits" << std::endl; break; default: _state=PACKETSTATE_HEADER;// For now, do the next page @@ -221,19 +233,31 @@ Packet* PacketMag::GetPacket(Packet* p) /** Is there a packet ready to go? * If the ready flag is set * and the priority count allows a packet to go out + * @param force - If true AND if the next packet is being held back due to priority, send the packet anyway */ -bool PacketMag::IsReady() +bool PacketMag::IsReady(bool force) { - // This happens unless we have just sent out a header - if (GetEvent(EVENT_FIELD)) + bool result=false; + // We can always send something unless + // 1) We have just sent out a header and are waiting on a new field + // 2) There are no pages + if ( ((GetEvent(EVENT_FIELD)) || (_state==PACKETSTATE_HEADER)) && (_pageSet->size()>0)) { // If we send a header we want to wait for this to get set GetEvent(EVENT_FIELD) _priorityCount--; - if (_priorityCount==0) + if (_priorityCount==0 || force) { _priorityCount=_priority; - return true; + result=true; } } - return false; + /* + std::cerr << "[PacketMag::IsReady] exits." + " mag=" << _magNumber << + " force=" << force << + " result=" << result << + " EVENT_FIELD=" << GetEvent(EVENT_FIELD) << + std::endl; + */ + return result; }; diff --git a/packetmag.h b/packetmag.h index ae172a2..efd0fef 100644 --- a/packetmag.h +++ b/packetmag.h @@ -24,8 +24,7 @@ class PacketMag : public PacketSource */ Packet* GetPacket(Packet* p) override; - bool IsReady(); - + bool IsReady(bool force=false); protected: diff --git a/packetsource.h b/packetsource.h index 905ddf6..61357cc 100644 --- a/packetsource.h +++ b/packetsource.h @@ -89,8 +89,11 @@ class PacketSource */ virtual Packet* GetPacket(Packet* p)=0; - /** Is there a packet ready to go? */ - virtual bool IsReady(){return _readyFlag;}; + /** Is there a packet ready to go? + * @param force - If true and the next packet's priority is holding it, then allow the packet to go anyway. Default false. + * @return true if there is a packet ready to go. + */ + virtual bool IsReady(bool force=false)=0; // {return _readyFlag;}; /** Report that an event happened */ void SetEvent(Event event); // All packet sources can use the same code diff --git a/pagelist.cpp b/pagelist.cpp index b5ab25c..325ca62 100644 --- a/pagelist.cpp +++ b/pagelist.cpp @@ -8,8 +8,8 @@ PageList::PageList(Configure *configure) : _configure(configure) { for (int i=0;i<8;i++) - _mag[i]=NULL; - if (_configure==NULL) + _mag[i]=nullptr; + if (_configure==nullptr) { std::cerr << "NULL configuration object" << std::endl; return; diff --git a/service.cpp b/service.cpp index 603ae4e..974a841 100644 --- a/service.cpp +++ b/service.cpp @@ -43,44 +43,67 @@ int Service::run() std::cerr << "[Service::worker]This is the worker process" << std::endl; std::list::const_iterator iterator=_Sources.begin(); // Iterator for packet sources + std::list::const_iterator first; // Save the first so we know if we have looped. + vbit::Packet* pkt=new vbit::Packet(8,25," "); // This just allocates storage. static vbit::Packet* filler=new vbit::Packet(8,25," "); // @todo Again, we should have a pre-prepared quiet packet to avoid eating the heap - std::cerr << "[Service::worker]Loop starts" << std::endl; + std::cerr << "[Service::run]Loop starts" << std::endl; while(1) // normal { + std::cerr << "[Service::run]iterates. VBI line=" << (int) _lineCounter << " (int) field=" << (int) _fieldCounter << std::endl; // If counters (or other trigger) causes an event then send the events _updateEvents(); // Must only call this ONCE per transmitted row // Iterate through the packet sources until we get a packet to transmit vbit::PacketSource* p; + first=iterator; + bool force=false; + uint8_t sourceCount=0; + uint8_t listSize=_Sources.size(); do { + // Loop back to the first source if (iterator==_Sources.end()) { iterator=_Sources.begin(); - // @todo Count iterations here and break out with a filler to prevent a deadlock } + + // If we have tried all sources with and without force, then break out with a filler to prevent a deadlock + if (sourceCount>listSize*2) + { + p=nullptr; + std::cerr << "[Service::run] No packet available for this line" << std::endl; + break; + } + + // If we have gone around once and got nothing, then force sources to go if possible. + if (sourceCount>listSize) + { + force=true; + } + + // Get the packet source p=(*iterator); ++iterator; - } - while (!p->IsReady()); - - p->GetPacket(pkt); // @todo Don't annoy Alistair by putting this bug back in + sourceCount++; // Count how many sources we tried. + } + while (!p->IsReady(force)); - // Transmit the packet - // @todo Does this code make sense? - if (pkt!=NULL) // How could this be NULL? After the last packet of a page has been sent. + if (p) { + // Big assumption here. pkt must always return a valid packet + // Which means that GetPacket is not allowed to fail. Is this correct? + p->GetPacket(pkt); // I know this was the original bug, but really don't want to create it dynamically std::cout.write(pkt->tx(), 42); // Transmit the packet - using cout.write to ensure writing 42 bytes even if it contains a null. } else { std::cout << filler->tx(); - } // blocked + } } // while return 99; // don't really want to return anything @@ -88,6 +111,7 @@ int Service::run() void Service::_updateEvents() { + static int seconds=0; // Step the counters _lineCounter++; if (_lineCounter%LINESPERFIELD==0) @@ -97,7 +121,12 @@ void Service::_updateEvents() if (_fieldCounter>=50) { _fieldCounter=0; + seconds++; + std::cerr << "Seconds=" << seconds << std::endl; // Could implement a seconds counter here if we needed it + + + // if (seconds>30) exit(0); // JUST FOR DEBUGGING!!!! Must remove } // New field, so set the FIELD event in all the sources. for (std::list::const_iterator iterator = _Sources.begin(), end = _Sources.end(); iterator != end; ++iterator) diff --git a/ttxpagestream.h b/ttxpagestream.h index 85e5db7..750d9c1 100644 --- a/ttxpagestream.h +++ b/ttxpagestream.h @@ -65,12 +65,12 @@ class TTXPageStream : public TTXPage /** Set the time when this carousel expires * ...which is the current time plus the cycle time */ - inline void SetTransitionTime(){_transitionTime=time(0)+m_cycletimeseconds;}; + inline void SetTransitionTime(){_transitionTime=time(nullptr)+m_cycletimeseconds;}; /** Used to time carousels * @return true if it is time to change carousel page */ - inline bool Expired(){return _transitionTime<=time(0);}; + inline bool Expired(){return _transitionTime<=time(nullptr);}; /** Step to the next page in a carousel * Updates _CarouselPage;