Merge pull request #5 from ZIMO-Elektronik/standardize_rcn217

Standardize rcn217

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## 0.29.0
+- Standardize tip-off search (use broadcast XF2 instead of reserved address 253)
+- Standardize track voltage datagram (5V offset)
+
 ## 0.28.1
 - Remove arbitrary RMT preamble limit
 

CMakeLists.txt

@@ -55,6 +55,8 @@ target_compile_definitions(
 
 if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME)
   target_include_directories(DCC INTERFACE include)
+  target_compile_options(DCC
+                         INTERFACE -Werror;-Wno-error=deprecated-declarations)
 else()
   target_include_directories(DCC SYSTEM INTERFACE include)
 endif()

TODO.md

@@ -1,4 +1,3 @@
 - Consist address is never used, although it should be sent in channel 1 in case it's active
-- Use std::span instead of raw pointers
 - Literal for converting CV number to index? The "- 1u" everything is fucking ugly
 - dcc::tx::CrtpBase currently pops it's deque at a bad time. Although the design or inplace_deque guarantees that it's not UB it looks funky.

examples/repl/decoder.cpp

@@ -27,12 +27,12 @@ void Decoder::emergencyStop(uint32_t addr) {
 }
 
 void Decoder::function(uint32_t addr, uint32_t mask, uint32_t state) {
-  auto const f_high{std::bit_width(mask) - 1u};
+  auto const f_high{std::bit_width(mask) - 1};
   auto const f_low{std::countr_zero(mask)};
   cli::Cli::cout() << "Address " << addr << ": f" << f_high << "-"
                    << "f" << f_low << " = "
                    << "0b";
-  for (int i{f_high}; i >= f_low; --i)
+  for (auto i{f_high}; i >= f_low; --i)
     cli::Cli::cout() << static_cast<bool>(state & (1u << i));
   cli::Cli::cout() << PROMPTENDL;
 }
@@ -43,7 +43,7 @@ void Decoder::serviceAck() {}
 
 void Decoder::transmitBiDi(std::span<uint8_t const>) {}
 
-uint8_t Decoder::readCv(uint32_t cv_addr, uint8_t byte) {
+uint8_t Decoder::readCv(uint32_t cv_addr, [[maybe_unused]] uint8_t byte) {
   cli::Cli::cout() << "Read CV byte " << cv_addr
                    << "==" << static_cast<uint32_t>(_cvs[cv_addr])
                    << PROMPTENDL;
@@ -56,15 +56,18 @@ uint8_t Decoder::writeCv(uint32_t cv_addr, uint8_t byte) {
   return _cvs[cv_addr] = byte;
 }
 
-bool Decoder::readCv(uint32_t cv_addr, bool bit, uint32_t pos) {
+bool Decoder::readCv(uint32_t cv_addr,
+                     [[maybe_unused]] bool bit,
+                     uint32_t pos) {
   auto const red_bit{static_cast<bool>(_cvs[cv_addr] & (1u << pos))};
   cli::Cli::cout() << "Read CV bit " << cv_addr << ":" << pos << "==" << red_bit
                    << PROMPTENDL;
   return red_bit;
 }
 
 bool Decoder::writeCv(uint32_t cv_addr, bool bit, uint32_t pos) {
-  _cvs[cv_addr] = (_cvs[cv_addr] & ~(1u << pos)) | (bit << pos);
+  _cvs[cv_addr] =
+    static_cast<uint8_t>((_cvs[cv_addr] & ~(1u << pos)) | (bit << pos));
   auto const red_bit{static_cast<bool>(_cvs[cv_addr] & (1u << pos))};
   cli::Cli::cout() << "Write CV bit " << cv_addr << ":" << pos << "=" << red_bit
                    << PROMPTENDL;

include/dcc/address.hpp

@@ -44,66 +44,69 @@ struct Address {
     Accessory,       ///< Address is accessory decoder
     Long,            ///< Address is long
     Reserved,        ///< Address is reserved
-    TipOffSearch,    ///< Address for tip-off search
-    ExtendedPacket,  ///< Address is extended packet
+    DataTransfer,    ///< Address for data transfer
+    AutomaticLogon,  ///< Address is automatic logon
     IdleSystem       ///< Address for system commands
   } type{};
 };
 
 /// Decode address
 ///
-/// \param  data  Pointer to data
+/// \tparam InputIt std::input_iterator
+/// \param  first   Beginning of the range to decode from
 /// \return Address
-constexpr Address decode_address(uint8_t const* data) {
+template<std::input_iterator InputIt>
+constexpr Address decode_address(InputIt first) {
   // 0
-  if (data[0uz] < 1u) return {data[0uz], Address::Broadcast};
+  if (*first < 1u) return {*first, Address::Broadcast};
   // 1-127
-  else if (data[0uz] < 128u) return {data[0uz], Address::Short};
+  else if (*first < 128u) return {*first, Address::Short};
   // 128-191
-  else if (data[0uz] < 192u)
-    return {data[0uz], Address::Accessory};  // TODO most likely wrong?
+  else if (*first < 192u)
+    return {*first, Address::Accessory};  // TODO most likely wrong?
   // 192-231
-  else if (data[0uz] < 232u) {
-    auto const addr{(static_cast<uint32_t>(data[0uz] << 8u) | data[1uz]) &
-                    0x3FFFu};
-    return {static_cast<Address::value_type>(addr), Address::Long};
+  else if (*first < 232u) {
+    auto const a13_8{*first++};
+    auto const a7_0{*first};
+    return {static_cast<Address::value_type>(((a13_8 << 8u) | a7_0) & 0x3FFFu),
+            Address::Long};
   }
   // 232-252
-  else if (data[0uz] < 253u)
-    return {data[0uz], Address::Reserved};
+  else if (*first < 253u)
+    return {*first, Address::Reserved};
   // 253
-  else if (data[0uz] < 254u) return {data[0uz], Address::TipOffSearch};
+  else if (*first < 254u) return {*first, Address::DataTransfer};
   // 254
-  else if (data[0uz] < 255u) return {data[0uz], Address::ExtendedPacket};
+  else if (*first < 255u) return {*first, Address::AutomaticLogon};
   // 255
-  else return {data[0uz], Address::IdleSystem};
+  else return {*first, Address::IdleSystem};
 }
 
 /// Encode address
 ///
-/// \tparam OutputIt
-/// \param  addr
-/// \param  data
-/// \return Address
+/// \tparam OutputIt  std::output_iterator<uint8_t>
+/// \param  addr      Address
+/// \param  first     Beginning of the range to encode to
+/// \return Iterator to the element that follows the last element encoded
 template<std::output_iterator<uint8_t> OutputIt>
-constexpr OutputIt encode_address(Address addr, OutputIt data) {
+constexpr OutputIt encode_address(Address addr, OutputIt first) {
   switch (addr.type) {
     case Address::UnknownService: break;
-    case Address::Broadcast: *data++ = 0u; break;
-    case Address::Short: *data++ = static_cast<uint8_t>(addr); break;
+    case Address::Broadcast: *first++ = 0u; break;
+    case Address::Short: *first++ = static_cast<uint8_t>(addr); break;
     case Address::Accessory:
       // TODO (see decode_address)
       break;
     case Address::Long:
-      *data++ = 0b1100'0000u | addr >> 8u;
-      *data++ = static_cast<uint8_t>(addr);
+      *first++ = static_cast<uint8_t>(0b1100'0000u | addr >> 8u);
+      *first++ = static_cast<uint8_t>(addr);
       break;
     case Address::Reserved: assert(false); break;
-    case Address::TipOffSearch: *data++ = 253u; break;
-    case Address::ExtendedPacket: *data++ = 254u; break;
-    case Address::IdleSystem: *data++ = 255u; break;
+    case Address::DataTransfer: *first++ = 253u; break;
+    case Address::AutomaticLogon: *first++ = 254u; break;
+    case Address::IdleSystem: *first++ = 255u; break;
   }
-  return data;
+  return first;
 }
 
 }  // namespace dcc

include/dcc/instruction.hpp

@@ -29,10 +29,12 @@ enum class Instruction : uint8_t {
 
 /// Decode instruction
 ///
-/// \param  data  Pointer to instruction data
+/// \tparam InputIt std::input_iterator
+/// \param  first   Beginning of the range to decode from
 /// \return Instruction
-constexpr Instruction decode_instruction(uint8_t const* data) {
-  switch (*data & 0xF0u) {
+template<std::input_iterator InputIt>
+constexpr Instruction decode_instruction(InputIt first) {
+  switch (*first & 0xF0u) {
     case 0b0000'0000u: return Instruction::DecoderControl;
     case 0b0001'0000u: return Instruction::ConsistControl;
     case 0b0010'0000u: [[fallthrough]];
@@ -51,7 +53,7 @@ constexpr Instruction decode_instruction(uint8_t const* data) {
     case 0b1111'0000u: return Instruction::CvShort;
     default: break;
   }
-  if (*data == 0b1111'1110u) return Instruction::Logon;
+  if (*first == 0b1111'1110u) return Instruction::Logon;
   return Instruction::UnknownService;
 }