decoder/encoder: simplification of code.

- Useless tests removed
- Bit operation changed to more readable ones
- Handle correctly mask to decode signal on 1 byte or in-between bytes more clearly.
- using static_cast now instead of C casting method.
- Avoid manual vector initialization and using default constructors instead.
- Avoid using intermediate variables when this isn't necessary.

Change-Id: I049d65f460109772b57df7572bdac8e6500242e0
Signed-off-by: Romain Forlot <romain.forlot@iot.bzh>

diff --git a/low-can-binding/can/can-decoder.cpp b/low-can-binding/can/can-decoder.cpp
index 7e60051..e9d0155 100644 (file)
--- a/low-can-binding/can/can-decoder.cpp
+++ b/low-can-binding/can/can-decoder.cpp
@@ -146,8 +146,9 @@ openxc_DynamicField decoder_t::decode_bytes(signal_t& signal, std::shared_ptr<me
        uint32_t length = message->get_length();
        uint32_t bit_position = signal.get_bit_position();
        uint32_t bit_size = signal.get_bit_size();
+
        std::vector<uint8_t> new_data = std::vector<uint8_t>();
-       new_data.reserve(bit_size << 3);
+       new_data.reserve((bit_size / 8) + 1);
 
        int new_start_byte = 0;
        int new_end_byte = 0;
@@ -157,9 +158,7 @@ openxc_DynamicField decoder_t::decode_bytes(signal_t& signal, std::shared_ptr<me
        converter_t::signal_to_bits_bytes(bit_position, bit_size, new_start_byte, new_end_byte, new_start_bit, new_end_bit);
 
        if(new_end_byte >= length)
-       {
                new_end_byte = length-1;
-       }
 
        if(new_start_byte >= length)
        {
@@ -167,49 +166,21 @@ openxc_DynamicField decoder_t::decode_bytes(signal_t& signal, std::shared_ptr<me
                return decoded_value;
        }
 
-       uint8_t first = data[new_start_byte];
-       int mask_first = 0;
-       for(i=new_start_bit;i<8;i++)
-       {
-               mask_first = mask_first | (1 << i);
-       }
-
-       uint8_t mask_first_v = 0;
-       if(mask_first > 255)
-       {
-               AFB_ERROR("Error mask decode bytes");
-       }
-       else
-       {
-               mask_first_v = (uint8_t)mask_first;
-       }
-
-       data[new_start_byte]=first&mask_first_v;
-
-       uint8_t last = data[new_end_byte];
-       int mask_last = 0;
-       for(i=0;i<=new_end_bit;i++)
-       {
-               mask_last = mask_last | (1 << (7-i));
-       }
+       uint8_t mask_first_v = static_cast<uint8_t>(0xFF << new_start_bit);
+       uint8_t mask_last_v = static_cast<uint8_t>(0xFF >> (7 - new_end_bit));
 
-       uint8_t mask_last_v = 0;
-       if(mask_last > 255)
+       if(new_start_byte == new_end_byte)
        {
-               AFB_ERROR("Error mask decode bytes");
+               data[new_start_byte] = data[new_start_byte] & (mask_first_v & mask_last_v);
        }
        else
        {
-               mask_last_v = (uint8_t)mask_last;
+               data[new_start_byte] = data[new_start_byte] & mask_first_v;
+               data[new_end_byte] = data[new_end_byte] & mask_last_v;
        }
 
-       data[new_end_byte]=last&mask_last_v;
-
-
-       for(i=new_start_byte;i<=new_end_byte;i++)
-       {
+       for(i=new_start_byte ; i <= new_end_byte ; i++)
                new_data.push_back(data[i]);
-       }
 
        decoded_value = build_DynamicField(new_data);
 
@@ -267,9 +238,8 @@ openxc_DynamicField decoder_t::decode_boolean(signal_t& signal, std::shared_ptr<
 
        // Don't send if they is no changes
        if ((signal.get_last_value() == value && !signal.get_send_same()) || !*send )
-       {
                *send = false;
-       }
+
        signal.set_last_value(value);
 
 

diff --git a/low-can-binding/can/can-encoder.cpp b/low-can-binding/can/can-encoder.cpp
index 36be668..2e79148 100644 (file)
--- a/low-can-binding/can/can-encoder.cpp
+++ b/low-can-binding/can/can-encoder.cpp
@@ -35,96 +35,47 @@ void encoder_t::encode_data(std::shared_ptr<signal_t> sig, std::vector<uint8_t>
 {
        uint32_t bit_size = sig->get_bit_size();
        uint32_t bit_position = sig->get_bit_position();
+       float factor_v = factor ? sig->get_factor() : 1;
+       float offset_v = offset ? sig->get_offset() : 0;
+
        int new_start_byte = 0;
        int new_end_byte = 0;
        uint8_t new_start_bit = 0;
        uint8_t new_end_bit = 0;
 
        converter_t::signal_to_bits_bytes(bit_position, bit_size, new_start_byte, new_end_byte, new_start_bit, new_end_bit);
-
-       int len_signal_bytes_tmp = new_end_byte - new_start_byte + 1;
-
-       uint8_t len_signal_bytes = 0;
-       if(len_signal_bytes_tmp > 255)
-       {
-               AFB_ERROR("Error signal %s too long", sig->get_name().c_str());
-       }
-       else
-       {
-               len_signal_bytes = (uint8_t) len_signal_bytes_tmp;
-       }
-/*
-       if(new_start_bit > 255)
-       {
-               AFB_ERROR("Error signal %s too long", sig->get_name().c_str());
-       }
-*/
-       uint8_t new_bit_size = 0;
-       if(bit_size > 255)
-       {
-               AFB_ERROR("Error signal %s to long bit size", sig->get_name().c_str());
-       }
-       else
-       {
-               new_bit_size = (uint8_t) bit_size;
-       }
-
-       uint8_t data_signal[len_signal_bytes] = {0};
-       float factor_v = 1;
-       if(factor)
-       {
-               factor_v = sig->get_factor();
-       }
-
-       float offset_v = 0;
-       if(factor)
-       {
-               offset_v = sig->get_offset();
-       }
+       std::vector<uint8_t> data_signal(new_end_byte - new_start_byte + 1);
 
        if(filter)
        {
-               uint8_t tmp = 0;
-               int j=0;
-               for(int i=0;i<new_bit_size;i++)
-               {
-                       int mask = 1 << ((i%8)+new_start_bit);
-
-                       uint8_t mask_v = 0;
-                       if(mask > 255)
-                       {
-                               AFB_ERROR("Error mask too large");
-                       }
-                       else
-                       {
-                               mask_v = (uint8_t) mask;
-                       }
-                       tmp = tmp|mask_v;
+               for (auto& elt: data_signal)
+                       elt = 0xFF;
+               uint8_t mask_first_v = static_cast<uint8_t>(0xFF << new_start_bit);
+               uint8_t mask_last_v = static_cast<uint8_t>(0xFF >> (7 - new_end_bit));
 
-                       if(i%8 == 7)
-                       {
-                               data_signal[j] = tmp;
-                               tmp = 0;
-                               j++;
-                       }
+               if(new_start_byte == new_end_byte)
+               {
+                       data_signal[0] = mask_first_v & mask_last_v;
+               }
+               else
+               {
+                       data_signal[0] = mask_first_v;
+                       data_signal[new_end_byte - new_start_byte] = mask_last_v;
                }
-               data_signal[j]=tmp;
        }
        else
        {
-               bitfield_encode_float(  sig->get_last_value(),
-                                               new_start_bit,
-                                               new_bit_size,
-                                               factor_v,
-                                               offset_v,
-                                               data_signal,
-                                               len_signal_bytes);
+               bitfield_encode_float(sig->get_last_value(),
+                                     new_start_bit,
+                                     bit_size,
+                                     factor_v,
+                                     offset_v,
+                                     data_signal.data(),
+                                     bit_size);
        }
 
        for(size_t i = new_start_byte; i <= new_end_byte ; i++)
-       {
                data[i] = data[i] | data_signal[i-new_start_byte];
-       }
 }
 
 /**
@@ -139,17 +90,12 @@ void encoder_t::encode_data(std::shared_ptr<signal_t> sig, std::vector<uint8_t>
  */
 message_t* encoder_t::build_frame(const std::shared_ptr<signal_t>& signal, uint64_t value, message_t *message, bool factor, bool offset)
 {
-       signal->set_last_value((float)value);
-       std::vector<uint8_t> data;
-       for(int i = 0; i<message->get_length();i++)
-       {
-               data.push_back(0);
-       }
+       signal->set_last_value(static_cast<float>(value));
+       std::vector<uint8_t> data(message->get_length(), 0);
 
        for(const auto& sig: signal->get_message()->get_signals())
-       {
                encode_data(sig, data, false, factor, offset);
-       }
+
        message->set_data(data);
        return message;
 }
@@ -222,24 +168,19 @@ message_t* encoder_t::build_message(const std::shared_ptr<signal_t>& signal, uin
 message_t* encoder_t::build_one_frame_message(const std::shared_ptr<signal_t>& signal, uint64_t value, message_t *message)
 {
        signal->set_last_value((float)value);
-       uint8_t data_tab[message->get_length()];
        std::vector<uint8_t> data;
+       data.reserve(message->get_length());
 
        for(const auto& sig: signal->get_message()->get_signals())
        {
                float last_value = sig->get_last_value();
                bitfield_encode_float(last_value,
-                                       sig->get_bit_position(),
-                                       sig->get_bit_size(),
+                                       static_cast<uint8_t>(sig->get_bit_position()),
+                                       static_cast<uint8_t>(sig->get_bit_size()),
                                        sig->get_factor(),
                                        sig->get_offset(),
-                                       data_tab,
-                                       (uint8_t)message->get_length());
-       }
-
-       for (size_t i = 0; i < (uint8_t) message->get_length(); i++)
-       {
-               data.push_back(data_tab[i]);
+                                       data.data(),
+                                       static_cast<uint8_t>(message->get_length()));
        }
 
        message->set_data(data);
@@ -257,38 +198,25 @@ message_t* encoder_t::build_one_frame_message(const std::shared_ptr<signal_t>& s
 message_t* encoder_t::build_multi_frame_message(const std::shared_ptr<signal_t>& signal, uint64_t value, message_t *message)
 {
        signal->set_last_value((float)value);
-       std::vector<uint8_t> data;
-
+       int nb_of_frame;
        uint32_t msgs_len = signal->get_message()->get_length(); // multi frame - number of bytes
-       int number_of_frame = (int) msgs_len / 8;
+       int max_dlen = signal->get_message()->get_flags() & CAN_PROTOCOL_WITH_FD_FRAME ? CANFD_MAX_DLEN : CAN_MAX_DLEN;
 
-       uint8_t data_tab[number_of_frame][8] = {0};
+       nb_of_frame = (int) msgs_len / max_dlen;
+       std::vector<uint8_t> data_tab;
+       data_tab.reserve(nb_of_frame * max_dlen);
 
        for(const auto& sig: signal->get_message()->get_signals())
-       {
-
-               int frame_position = (int) sig->get_bit_position() / 64;
-               float last_value = sig->get_last_value();
-               uint8_t bit_position = sig->get_bit_position() - ((uint8_t)(64 * frame_position));
-
-               bitfield_encode_float(last_value,
-                                       bit_position,
-                                       sig->get_bit_size(),
+               // TODO: find a way to handle huge signal
+               bitfield_encode_float(sig->get_last_value(),
+                                       sig->get_bit_position() % 64,
+                                       (uint8_t) sig->get_bit_size(),
                                        sig->get_factor(),
                                        sig->get_offset(),
-                                       data_tab[frame_position],
-                                       8);
-       }
+                                       data_tab.data() + (sig->get_bit_position() / 64),
+                                       (nb_of_frame * max_dlen) - (sig->get_bit_position() / 64));
 
-       for (size_t i = 0; i < number_of_frame; i++)
-       {
-               for(size_t j = 0; j < 8 ; j++)
-               {
-                       data.push_back(data_tab[i][j]);
-               }
-       }
-
-       message->set_data(data);
+       message->set_data(data_tab);
        return message;
 }