Uses proper helper function for decoding Node ID. (#616)

Fixes a todo in the TractionCVSpace on how the node IDs were decoded into DCC address.
This decoding now uses the helper function from TractionDefs.

src/openlcb/TractionCvSpace.cxx

@@ -83,23 +83,37 @@ bool TractionCvSpace::set_node(Node *node)
         return false;
     }
     NodeID id = node->node_id();
-    /* NOTE(balazs.racz): It is difficult to figure out the DCC address given
-     * an abstract NMRAnet node. Here we hardcode the reserved DCC node ID
-     * space, which is not a great solution. */
-    if ((id & 0xFFFF00000000ULL) == TractionDefs::NODE_ID_DCC)
+    if ((id & 0xFFFF00000000ULL) != TractionDefs::NODE_ID_DCC)
     {
-        uint16_t new_address = id & 0xFFFFU;
-        if (dccAddress_ != new_address)
-        {
-            dccAddress_ = new_address;
-            errorCode_ = ERROR_NOOP;
-        }
-        return true;
+        currId_ = 0;
+        return false;
     }
-    else
+    if ((id & 0xFFFF) == currId_)
     {
+        return true;
+    }
+    currId_ = (id & 0xFFFF);
+    dcc::TrainAddressType type;
+    uint32_t addr;
+    if (!TractionDefs::legacy_address_from_train_node_id(id, &type, &addr)) {
+        currId_ = 0;
         return false;
     }
+    switch(type) {
+    case dcc::TrainAddressType::DCC_LONG_ADDRESS:
+        dccAddressNum_ = addr;
+        dccIsLong_ = true;
+        break;
+    case dcc::TrainAddressType::DCC_SHORT_ADDRESS:
+        dccAddressNum_ = addr;
+        dccIsLong_ = false;
+        break;
+    default:
+        currId_ = 0;
+        return false;
+    }
+    errorCode_ = ERROR_NOOP;
+    return true;
 }
 
 const unsigned TractionCvSpace::MAX_CV;
@@ -108,7 +122,7 @@ size_t TractionCvSpace::read(const address_t source, uint8_t *dst, size_t len,
     errorcode_t *error, Notifiable *again)
 {
     if (source == OFFSET_CV_INDEX) {
-        lastIndexedNode_ = dccAddress_;
+        lastIndexedNode_ = currId_;
         uint8_t* lastcv = (uint8_t*)&lastIndexedCv_;
         if (len > 0) dst[0] = lastcv[3];
         if (len > 1) dst[1] = lastcv[2];
@@ -119,7 +133,7 @@ size_t TractionCvSpace::read(const address_t source, uint8_t *dst, size_t len,
     uint32_t cv = -1;
     if (source == OFFSET_CV_VALUE || source == OFFSET_CV_VERIFY_RESULT)
     {
-        if (dccAddress_ != lastIndexedNode_)
+        if (currId_ != lastIndexedNode_)
         {
             *error = Defs::ERROR_PERMANENT;
             return 0;
@@ -278,15 +292,13 @@ StateFlowBase::Action TractionCvSpace::fill_read1_packet()
 {
     auto *b = get_allocation_result(track_);
     b->data()->start_dcc_packet();
-    /** @TODO(balazs.racz) here we make bad assumptions about how to decide
-     * between long and short addresses */
-    if (dccAddress_ >= 0x80)
+    if (dccIsLong_)
     {
-        b->data()->add_dcc_address(dcc::DccLongAddress(dccAddress_ & 0x3FFF));
+        b->data()->add_dcc_address(dcc::DccLongAddress(dccAddressNum_));
     }
     else
     {
-        b->data()->add_dcc_address(dcc::DccShortAddress(dccAddress_));
+        b->data()->add_dcc_address(dcc::DccShortAddress(dccAddressNum_));
     }
     b->data()->add_dcc_pom_read1(cvNumber_);
     b->data()->feedback_key = reinterpret_cast<uintptr_t>(this);
@@ -330,7 +342,7 @@ size_t TractionCvSpace::write(address_t destination, const uint8_t *src,
                               size_t len, errorcode_t *error, Notifiable *again)
 {
     if (destination == OFFSET_CV_INDEX) {
-        lastIndexedNode_ = dccAddress_;
+        lastIndexedNode_ = currId_;
         uint8_t* lastcv = (uint8_t*)&lastIndexedCv_;
         if (len > 0) lastcv[3] = src[0];
         if (len > 1) lastcv[2] = src[1];
@@ -344,7 +356,7 @@ size_t TractionCvSpace::write(address_t destination, const uint8_t *src,
         return 1;
     }
     if (destination == OFFSET_CV_VALUE) {
-        if (dccAddress_ != lastIndexedNode_) {
+        if (currId_ != lastIndexedNode_) {
             *error = Defs::ERROR_TEMPORARY;
             return 0;
         }
@@ -385,15 +397,13 @@ StateFlowBase::Action TractionCvSpace::fill_write1_packet()
 {
     auto *b = get_allocation_result(track_);
     b->data()->start_dcc_packet();
-    /** @todo(balazs.racz) here we make bad assumptions about how to decide
-     * between long and short addresses */
-    if (dccAddress_ >= 0x80)
+    if (dccIsLong_)
     {
-        b->data()->add_dcc_address(dcc::DccLongAddress(dccAddress_ & 0x3FFF));
+        b->data()->add_dcc_address(dcc::DccLongAddress(dccAddressNum_));
     }
     else
     {
-        b->data()->add_dcc_address(dcc::DccShortAddress(dccAddress_));
+        b->data()->add_dcc_address(dcc::DccShortAddress(dccAddressNum_));
     }
     b->data()->add_dcc_pom_write1(cvNumber_, cvData_);
     b->data()->feedback_key = reinterpret_cast<uintptr_t>(this);

src/openlcb/TractionCvSpace.hxx

@@ -118,9 +118,16 @@ private:
     MemoryConfigHandler *parent_;
     dcc::TrackIf *track_;
     dcc::RailcomHubFlow *railcomHub_;
-    uint16_t dccAddress_;
+    /// Last accepted DCC locomotive node ID (bottom 16 bits).
+    uint16_t currId_;
+    /// Numberic value of last used DCC address.
+    uint16_t dccAddressNum_ : 14;
+    /// 1 for long address, 0 for short address.
+    uint16_t dccIsLong_ : 1;
+    /// CV to read (0 to 1023).
     uint16_t cvNumber_;
-    uint8_t cvData_; //< data to read or write.
+    /// data read or data to write
+    uint8_t cvData_;
     uint8_t errorCode_ : 4;
     uint8_t numTry_ : 4;
     enum