From b77ee60ae34430fb25b05361ddde33565ab65678 Mon Sep 17 00:00:00 2001 From: Romain Forlot Date: Sat, 23 Nov 2019 12:42:01 +0100 Subject: [PATCH] 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 --- low-can-binding/can/can-decoder.cpp | 50 +++--------- low-can-binding/can/can-encoder.cpp | 158 ++++++++++-------------------------- 2 files changed, 53 insertions(+), 155 deletions(-) diff --git a/low-can-binding/can/can-decoder.cpp b/low-can-binding/can/can-decoder.cpp index 7e600518..e9d01557 100644 --- 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_ptrget_length(); uint32_t bit_position = signal.get_bit_position(); uint32_t bit_size = signal.get_bit_size(); + std::vector new_data = std::vector(); - 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= 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 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(0xFF << new_start_bit); + uint8_t mask_last_v = static_cast(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 36be668c..2e79148f 100644 --- 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 sig, std::vector { 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 data_signal(new_end_byte - new_start_byte + 1); if(filter) { - uint8_t tmp = 0; - int j=0; - for(int i=0;i 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(0xFF << new_start_bit); + uint8_t mask_last_v = static_cast(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 sig, std::vector */ message_t* encoder_t::build_frame(const std::shared_ptr& signal, uint64_t value, message_t *message, bool factor, bool offset) { - signal->set_last_value((float)value); - std::vector data; - for(int i = 0; iget_length();i++) - { - data.push_back(0); - } + signal->set_last_value(static_cast(value)); + std::vector 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, uin message_t* encoder_t::build_one_frame_message(const std::shared_ptr& signal, uint64_t value, message_t *message) { signal->set_last_value((float)value); - uint8_t data_tab[message->get_length()]; std::vector 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(sig->get_bit_position()), + static_cast(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(message->get_length())); } message->set_data(data); @@ -257,38 +198,25 @@ message_t* encoder_t::build_one_frame_message(const std::shared_ptr& s message_t* encoder_t::build_multi_frame_message(const std::shared_ptr& signal, uint64_t value, message_t *message) { signal->set_last_value((float)value); - std::vector 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 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; } -- 2.16.6