prepare v5.0.0, revert firmware interfacing changes

CHANGELOG.md

@@ -16,10 +16,22 @@ will consitute of a breaking change warranting a new major release:
 
 # [unreleased]
 
-# [v4.1.0]
+# [v6.0.0] to be released
+
+- Important bugfixes for PTME. See `q7s-package` CHANGELOG.
+
+# [v5.0.0] to be released
+
+v3.3.1 and all following version will now be moved to v5.0.0 with the additional changes listed
+here. This was done because the firmware update (v4.0.0) is not working right now and it is not
+known when and how it will be fixed. Because of that, all updates to make the SW work with the new
+firmware, which are limited to a few files will be moved to a dev branch so regular development
+compatible to the old firmware can continue.
 
 ## Changed
 
+- This version is compatible to the old firmware and some changes which only work with the new
+  firmware have been reverted.
 - Added `sync` syscall in graceful shutdown handler
 - Graceful shutdown is now performed by the reboot watchdog
 - There is now a separate file for the total reboot counter. The reboot watchdog has its own local

bsp_q7s/boardconfig/busConf.h

@@ -85,9 +85,13 @@ static constexpr char EN_RW_4[] = "enable_rw_4";
 static constexpr char RAD_SENSOR_CHIP_SELECT[] = "rad_sensor_chip_select";
 static constexpr char ENABLE_RADFET[] = "enable_radfet";
 
+static constexpr char PAPB_BUSY_SIGNAL_VC0[] = "papb_busy_signal_vc0";
 static constexpr char PAPB_EMPTY_SIGNAL_VC0[] = "papb_empty_signal_vc0";
+static constexpr char PAPB_BUSY_SIGNAL_VC1[] = "papb_busy_signal_vc1";
 static constexpr char PAPB_EMPTY_SIGNAL_VC1[] = "papb_empty_signal_vc1";
+static constexpr char PAPB_BUSY_SIGNAL_VC2[] = "papb_busy_signal_vc2";
 static constexpr char PAPB_EMPTY_SIGNAL_VC2[] = "papb_empty_signal_vc2";
+static constexpr char PAPB_BUSY_SIGNAL_VC3[] = "papb_busy_signal_vc3";
 static constexpr char PAPB_EMPTY_SIGNAL_VC3[] = "papb_empty_signal_vc3";
 
 static constexpr char PTME_RESETN[] = "ptme_resetn";

bsp_q7s/core/ObjectFactory.cpp

@@ -731,12 +731,20 @@ ReturnValue_t ObjectFactory::createCcsdsComponents(CcsdsComponentArgs& args) {
   // GPIO definitions of signals connected to the virtual channel interfaces of the PTME IP Core
   GpioCookie* gpioCookiePtmeIp = new GpioCookie;
   GpiodRegularByLineName* gpio = nullptr;
+  gpio = new GpiodRegularByLineName(q7s::gpioNames::PAPB_BUSY_SIGNAL_VC0, "PAPB VC0");
+  gpioCookiePtmeIp->addGpio(gpioIds::VC0_PAPB_BUSY, gpio);
   gpio = new GpiodRegularByLineName(q7s::gpioNames::PAPB_EMPTY_SIGNAL_VC0, "PAPB VC0");
   gpioCookiePtmeIp->addGpio(gpioIds::VC0_PAPB_EMPTY, gpio);
+  gpio = new GpiodRegularByLineName(q7s::gpioNames::PAPB_BUSY_SIGNAL_VC1, "PAPB VC1");
+  gpioCookiePtmeIp->addGpio(gpioIds::VC1_PAPB_BUSY, gpio);
   gpio = new GpiodRegularByLineName(q7s::gpioNames::PAPB_EMPTY_SIGNAL_VC1, "PAPB VC1");
   gpioCookiePtmeIp->addGpio(gpioIds::VC1_PAPB_EMPTY, gpio);
+  gpio = new GpiodRegularByLineName(q7s::gpioNames::PAPB_BUSY_SIGNAL_VC2, "PAPB VC2");
+  gpioCookiePtmeIp->addGpio(gpioIds::VC2_PAPB_BUSY, gpio);
   gpio = new GpiodRegularByLineName(q7s::gpioNames::PAPB_EMPTY_SIGNAL_VC2, "PAPB VC2");
   gpioCookiePtmeIp->addGpio(gpioIds::VC2_PAPB_EMPTY, gpio);
+  gpio = new GpiodRegularByLineName(q7s::gpioNames::PAPB_BUSY_SIGNAL_VC3, "PAPB VC3");
+  gpioCookiePtmeIp->addGpio(gpioIds::VC3_PAPB_BUSY, gpio);
   gpio = new GpiodRegularByLineName(q7s::gpioNames::PAPB_EMPTY_SIGNAL_VC3, "PAPB VC3");
   gpioCookiePtmeIp->addGpio(gpioIds::VC3_PAPB_EMPTY, gpio);
   gpio = new GpiodRegularByLineName(q7s::gpioNames::PTME_RESETN, "PTME RESETN",
@@ -745,14 +753,19 @@ ReturnValue_t ObjectFactory::createCcsdsComponents(CcsdsComponentArgs& args) {
   gpioChecker(args.gpioComIF.addGpios(gpioCookiePtmeIp), "PTME PAPB VCs");
 
   // Creating virtual channel interfaces
-  VirtualChannelIF* vc0 = new PapbVcInterface(&args.gpioComIF, gpioIds::VC0_PAPB_EMPTY,
+  VirtualChannelIF* vc0 =
+      new PapbVcInterface(&args.gpioComIF, gpioIds::VC0_PAPB_BUSY, gpioIds::VC0_PAPB_EMPTY,
                           q7s::UIO_PTME, q7s::uiomapids::PTME_VC0);
-  VirtualChannelIF* vc1 = new PapbVcInterface(&args.gpioComIF, gpioIds::VC1_PAPB_EMPTY,
+  VirtualChannelIF* vc1 =
+      new PapbVcInterface(&args.gpioComIF, gpioIds::VC1_PAPB_BUSY, gpioIds::VC1_PAPB_EMPTY,
                           q7s::UIO_PTME, q7s::uiomapids::PTME_VC1);
-  VirtualChannelIF* vc2 = new PapbVcInterface(&args.gpioComIF, gpioIds::VC2_PAPB_EMPTY,
+  VirtualChannelIF* vc2 =
+      new PapbVcInterface(&args.gpioComIF, gpioIds::VC2_PAPB_BUSY, gpioIds::VC2_PAPB_EMPTY,
                           q7s::UIO_PTME, q7s::uiomapids::PTME_VC2);
-  VirtualChannelIF* vc3 = new PapbVcInterface(&args.gpioComIF, gpioIds::VC3_PAPB_EMPTY,
+  VirtualChannelIF* vc3 =
+      new PapbVcInterface(&args.gpioComIF, gpioIds::VC3_PAPB_BUSY, gpioIds::VC3_PAPB_EMPTY,
                           q7s::UIO_PTME, q7s::uiomapids::PTME_VC3);
+
   // Creating ptme object and adding virtual channel interfaces
   Ptme* ptme = new Ptme(objects::PTME);
   ptme->addVcInterface(ccsds::VC0, vc0);

common/config/devices/gpioIds.h

@@ -96,9 +96,13 @@ enum gpioId_t {
 
   SPI_MUX,
   VC0_PAPB_EMPTY,
+  VC0_PAPB_BUSY,
   VC1_PAPB_EMPTY,
+  VC1_PAPB_BUSY,
   VC2_PAPB_EMPTY,
+  VC2_PAPB_BUSY,
   VC3_PAPB_EMPTY,
+  VC3_PAPB_BUSY,
   PTME_RESETN,
 
   PDEC_RESET,

linux/ipcore/PapbVcInterface.cpp

@@ -7,16 +7,20 @@
 
 #include "fsfw/serviceinterface/ServiceInterface.h"
 
-PapbVcInterface::PapbVcInterface(LinuxLibgpioIF* gpioComIF, gpioId_t papbEmptyId,
-                                 std::string uioFile, int mapNum)
-    : gpioComIF(gpioComIF), papbEmptyId(papbEmptyId), uioFile(std::move(uioFile)), mapNum(mapNum) {}
+PapbVcInterface::PapbVcInterface(LinuxLibgpioIF* gpioComIF, gpioId_t papbBusyId,
+                                 gpioId_t papbEmptyId, std::string uioFile, int mapNum)
+    : gpioComIF(gpioComIF),
+      papbBusyId(papbBusyId),
+      papbEmptyId(papbEmptyId),
+      uioFile(std::move(uioFile)),
+      mapNum(mapNum) {}
 
 PapbVcInterface::~PapbVcInterface() {}
 
 ReturnValue_t PapbVcInterface::initialize() {
   UioMapper uioMapper(uioFile, mapNum);
   ReturnValue_t result = uioMapper.getMappedAdress(const_cast<uint32_t**>(&vcBaseReg),
-                                                   UioMapper::Permissions::READ_WRITE);
+                                                   UioMapper::Permissions::WRITE_ONLY);
   if (result != returnvalue::OK) {
     return result;
   }
@@ -28,16 +32,63 @@ ReturnValue_t PapbVcInterface::write(const uint8_t* data, size_t size) {
   if (size < 4) {
     return returnvalue::FAILED;
   }
-  if (pollReadyForPacket()) {
+  if (pollInterfaceReadiness(0, true) == returnvalue::OK) {
     startPacketTransfer(ByteWidthCfg::ONE);
   } else {
     return DirectTmSinkIF::IS_BUSY;
   }
+  // TODO: This should work but does not.. :(
+  //  size_t idx = 0;
+  //  while (idx < size) {
+  //
+  //    nanosleep(&BETWEEN_POLL_DELAY, &remDelay);
+  //    if ((size - idx) < 4) {
+  //      *vcBaseReg = CONFIG_DATA_INPUT | (size - idx - 1);
+  //      usleep(1);
+  //    }
+  //    if (pollPapbBusySignal(2) == returnvalue::OK) {
+  //      //      vcBaseReg + DATA_REG_OFFSET + 3 = static_cast<uint8_t>(data + idx);
+  //      //      vcBaseReg + DATA_REG_OFFSET + 2 = static_cast<uint8_t>(data + idx + 1);
+  //      //      vcBaseReg + DATA_REG_OFFSET + 1 = static_cast<uint8_t>(data + idx + 2);
+  //      //      vcBaseReg + DATA_REG_OFFSET = static_cast<uint8_t>(data + idx + 3);
+  //
+  //      // std::memcpy((vcBaseReg + DATA_REG_OFFSET), data + idx , nextWriteSize);
+  //      *(vcBaseReg + DATA_REG_OFFSET) = *reinterpret_cast<const uint32_t*>(data + idx);
+  //      //uint8_t* byteReg = reinterpret_cast<uint8_t*>(vcBaseReg + DATA_REG_OFFSET);
+  //
+  //      //byteReg[0] = data[idx];
+  //      //byteReg[1] = data[idx];
+  //    } else {
+  //      abortPacketTransfer();
+  //      return returnvalue::FAILED;
+  //    }
+  //    // TODO: Change this after the bugfix. Right now, the PAPB ignores the content of the byte
+  //    // width configuration.5
+  //    // It's okay to increment by a larger amount for the last segment here, loop will be over
+  //    // in any case.
+  //    idx += 4;
+  //  }
   for (size_t idx = 0; idx < size; idx++) {
-    // if (pollInterfaceReadiness(2, false) == returnvalue::OK) {
+    // This delay is super-important, DO NOT REMOVE!
+    // Polling the GPIO or the config register too often messes up the scheduler.
+    // TODO: Maybe this should not be done like this. It would be better if there was a custom
+    // FPGA module which can accept packets and then takes care of dumping that packet into
+    // the PTME. DMA would be an ideal solution for this.
+    nanosleep(&BETWEEN_POLL_DELAY, &remDelay);
+    if (pollInterfaceReadiness(2, false) == returnvalue::OK) {
       *(vcBaseReg + DATA_REG_OFFSET) = static_cast<uint32_t>(data[idx]);
+    } else {
+      abortPacketTransfer();
+      return returnvalue::FAILED;
     }
+  }
+  nanosleep(&BETWEEN_POLL_DELAY, &remDelay);
+  if (pollInterfaceReadiness(2, false) == returnvalue::OK) {
     completePacketTransfer();
+  } else {
+    abortPacketTransfer();
+    return returnvalue::FAILED;
+  }
   return returnvalue::OK;
 }
 
@@ -47,33 +98,60 @@ void PapbVcInterface::startPacketTransfer(ByteWidthCfg initWidth) {
 
 void PapbVcInterface::completePacketTransfer() { *vcBaseReg = CONFIG_END; }
 
-bool PapbVcInterface::pollReadyForPacket() const {
+ReturnValue_t PapbVcInterface::pollInterfaceReadiness(uint32_t maxPollRetries,
+                                                      bool checkReadyState) const {
+  uint32_t busyIdx = 0;
+  nextDelay.tv_nsec = FIRST_DELAY_PAPB_POLLING_NS;
+
+  while (true) {
     // Check if PAPB interface is ready to receive data. Use the configuration register for this.
     // Bit 5, see PTME ptme_001_01-0-7-r2 Table 31.
     uint32_t reg = *vcBaseReg;
-  // bool busy = (reg >> 5) & 0b1;
-  return (reg >> 6) & 0b1;
+    bool busy = (reg >> 5) & 0b1;
+    bool ready = (reg >> 6) & 0b1;
+    if (not busy) {
+      return returnvalue::OK;
+    }
+    if (checkReadyState and not ready) {
+      return PAPB_BUSY;
     }
 
-bool PapbVcInterface::isVcInterfaceBufferEmpty() {
+    busyIdx++;
+    if (busyIdx >= maxPollRetries) {
+      return PAPB_BUSY;
+    }
+
+    // Ignore signal handling here for now.
+    nanosleep(&nextDelay, &remDelay);
+    // Adaptive delay.
+    if (nextDelay.tv_nsec * 2 <= MAX_DELAY_PAPB_POLLING_NS) {
+      nextDelay.tv_nsec *= 2;
+    }
+  }
+  return returnvalue::OK;
+}
+
+void PapbVcInterface::isVcInterfaceBufferEmpty() {
   ReturnValue_t result = returnvalue::OK;
   gpio::Levels papbEmptyState = gpio::Levels::HIGH;
 
   result = gpioComIF->readGpio(papbEmptyId, papbEmptyState);
 
   if (result != returnvalue::OK) {
-    sif::error << "PapbVcInterface::isVcInterfaceBufferEmpty: Failed to read papb empty signal"
+    sif::warning << "PapbVcInterface::isVcInterfaceBufferEmpty: Failed to read papb empty signal"
                  << std::endl;
-    return true;
+    return;
   }
 
   if (papbEmptyState == gpio::Levels::HIGH) {
-    return true;
+    sif::debug << "PapbVcInterface::isVcInterfaceBufferEmpty: Buffer is empty" << std::endl;
+  } else {
+    sif::debug << "PapbVcInterface::isVcInterfaceBufferEmpty: Buffer is not empty" << std::endl;
   }
-  return false;
+  return;
 }
 
-bool PapbVcInterface::isBusy() const { return not pollReadyForPacket(); }
+bool PapbVcInterface::isBusy() const { return pollInterfaceReadiness(0, true) == PAPB_BUSY; }
 
 void PapbVcInterface::cancelTransfer() { abortPacketTransfer(); }
 

linux/ipcore/PapbVcInterface.h

@@ -30,7 +30,8 @@ class PapbVcInterface : public VirtualChannelIF {
    * @param uioFile       UIO file providing access to the PAPB bus
    * @param mapNum        Map number of UIO map associated with this virtual channel
    */
-  PapbVcInterface(LinuxLibgpioIF* gpioComIF, gpioId_t papbEmptyId, std::string uioFile, int mapNum);
+  PapbVcInterface(LinuxLibgpioIF* gpioComIF, gpioId_t papbBusyId, gpioId_t papbEmptyId,
+                  std::string uioFile, int mapNum);
   virtual ~PapbVcInterface();
 
   bool isBusy() const override;
@@ -82,6 +83,9 @@ class PapbVcInterface : public VirtualChannelIF {
   static constexpr long int MAX_DELAY_PAPB_POLLING_NS = 40;
 
   LinuxLibgpioIF* gpioComIF = nullptr;
+
+  /** Pulled to low when virtual channel not ready to receive data */
+  gpioId_t papbBusyId = gpio::NO_GPIO;
   /** High when external buffer memory of virtual channel is empty */
   gpioId_t papbEmptyId = gpio::NO_GPIO;
 
@@ -116,13 +120,13 @@ class PapbVcInterface : public VirtualChannelIF {
    *
    * @return  returnvalue::OK when ready to receive data else PAPB_BUSY.
    */
-  inline bool pollReadyForPacket() const;
+  inline ReturnValue_t pollInterfaceReadiness(uint32_t maxPollRetries, bool checkReadyState) const;
 
   /**
    * @brief   This function can be used for debugging to check whether there are packets in
    *          the packet buffer of the virtual channel or not.
    */
-  bool isVcInterfaceBufferEmpty();
+  void isVcInterfaceBufferEmpty();
 
   /**
    * @brief   This function sends a complete telemetry transfer frame data field (1105 bytes)

mission/com/PersistentLogTmStoreTask.cpp

@@ -42,7 +42,13 @@ ReturnValue_t PersistentLogTmStoreTask::performOperation(uint8_t opCode) {
     if (not someonesBusy) {
       TaskFactory::delayTask(100);
     } else if (vcBusyDuringDump) {
+      // TODO: Might not be necessary
+      sif::debug << "VC busy, delaying" << std::endl;
       TaskFactory::delayTask(10);
+    } else {
+      // TODO: Would be best to remove this, but not delaying here can lead to evil issues.
+      // Polling the PAPB of the PTME core too often leads to scheuduling issues.
+      TaskFactory::delayTask(2);
     }
   }
 }

mission/com/PersistentSingleTmStoreTask.cpp

@@ -24,7 +24,13 @@ ReturnValue_t PersistentSingleTmStoreTask::performOperation(uint8_t opCode) {
     if (not busy) {
       TaskFactory::delayTask(100);
     } else if (dumpContext.vcBusyDuringDump) {
+      sif::debug << "VC busy, delaying" << std::endl;
+      // TODO: Might not be necessary
       TaskFactory::delayTask(10);
+    } else {
+      // TODO: Would be best to remove this, but not delaying here can lead to evil issues.
+      // Polling the PAPB of the PTME core too often leads to scheuduling issues.
+      TaskFactory::delayTask(2);
     }
   }
 }