From 4126f1ab1fa8cc44b27157ccd148a6700bdd80fb Mon Sep 17 00:00:00 2001 From: Jared Boone Date: Thu, 20 Aug 2015 13:13:12 -0700 Subject: [PATCH] Change IPC to exchange data, not pointers. --- firmware/application/main.cpp | 11 ++-- firmware/application/receiver_model.cpp | 13 +--- firmware/application/ui_spectrum.hpp | 5 +- firmware/baseband/main.cpp | 85 +++++++++++-------------- firmware/common/message.hpp | 27 +++----- firmware/common/message_queue.cpp | 46 +++---------- firmware/common/message_queue.hpp | 15 +++-- 7 files changed, 76 insertions(+), 126 deletions(-) diff --git a/firmware/application/main.cpp b/firmware/application/main.cpp index 523c30f6..aaa26e5f 100755 --- a/firmware/application/main.cpp +++ b/firmware/application/main.cpp @@ -116,11 +116,12 @@ private: void handle_application_queue() { while( !shared_memory.application_queue.is_empty() ) { - auto message = shared_memory.application_queue.pop(); - - context.message_map.send(message); - - message->state = Message::State::Free; + std::array message_buffer; + const Message* const message = reinterpret_cast(message_buffer.data()); + const auto message_size = shared_memory.application_queue.pop(message_buffer.data(), message_buffer.size()); + if( message_size ) { + context.message_map.send(message); + } } } diff --git a/firmware/application/receiver_model.cpp b/firmware/application/receiver_model.cpp index f54ae86e..164d9ce8 100644 --- a/firmware/application/receiver_model.cpp +++ b/firmware/application/receiver_model.cpp @@ -147,8 +147,7 @@ void ReceiverModel::disable() { .decimation_factor = 1, } }; - shared_memory.baseband_queue.push(&message); - while( !message.is_free() ); + shared_memory.baseband_queue.push(message); radio::disable(); } @@ -187,10 +186,7 @@ void ReceiverModel::update_baseband_configuration() { radio::set_baseband_decimation_by(baseband_oversampling()); BasebandConfigurationMessage message { baseband_configuration }; - shared_memory.baseband_queue.push(&message); - - // Block until message is consumed, since we allocated it on the stack. - while( !message.is_free() ); + shared_memory.baseband_queue.push(message); if( baseband_configuration.mode == 3 ) { update_fsk_configuration(); @@ -222,8 +218,5 @@ static constexpr FSKConfiguration fsk_configuration_tpms_a = { void ReceiverModel::update_fsk_configuration() { FSKConfigurationMessage message { fsk_configuration_ais }; - shared_memory.baseband_queue.push(&message); - - // Block until message is consumed, since we allocated it on the stack. - while( !message.is_free() ); + shared_memory.baseband_queue.push(message); } diff --git a/firmware/application/ui_spectrum.hpp b/firmware/application/ui_spectrum.hpp index 28a2acd2..aba04a48 100644 --- a/firmware/application/ui_spectrum.hpp +++ b/firmware/application/ui_spectrum.hpp @@ -201,16 +201,15 @@ public: const ChannelSpectrum& spectrum ) { /* TODO: static_assert that message.spectrum.db.size() >= pixel_row.size() */ - const auto& db = *spectrum.db; std::array pixel_row; for(size_t i=0; i<120; i++) { - const auto pixel_color = spectrum_rgb3_lut[db[256 - 120 + i]]; + const auto pixel_color = spectrum_rgb3_lut[spectrum.db[256 - 120 + i]]; pixel_row[i] = pixel_color; } for(size_t i=120; i<240; i++) { - const auto pixel_color = spectrum_rgb3_lut[db[i - 120]]; + const auto pixel_color = spectrum_rgb3_lut[spectrum.db[i - 120]]; pixel_row[i] = pixel_color; } diff --git a/firmware/baseband/main.cpp b/firmware/baseband/main.cpp index c3d460e6..cf680bd3 100755 --- a/firmware/baseband/main.cpp +++ b/firmware/baseband/main.cpp @@ -273,10 +273,8 @@ private: AudioStatisticsMessage audio_stats_message; void post_channel_stats_message(const ChannelStatistics statistics) { - if( channel_stats_message.is_free() ) { - channel_stats_message.statistics = statistics; - shared_memory.application_queue.push(&channel_stats_message); - } + channel_stats_message.statistics = statistics; + shared_memory.application_queue.push(channel_stats_message); } void post_channel_spectrum_message(const buffer_c16_t data) { @@ -298,10 +296,8 @@ private: } void post_audio_stats_message(const AudioStatistics statistics) { - if( audio_stats_message.is_free() ) { - audio_stats_message.statistics = statistics; - shared_memory.application_queue.push(&audio_stats_message); - } + audio_stats_message.statistics = statistics; + shared_memory.application_queue.push(audio_stats_message); } }; @@ -585,11 +581,9 @@ private: const std::bitset<256>& payload, const size_t bits_received ) { - if( message.is_free() ) { - message.packet.payload = payload; - message.packet.bits_received = bits_received; - shared_memory.application_queue.push(&message); - } + message.packet.payload = payload; + message.packet.bits_received = bits_received; + shared_memory.application_queue.push(message); } }; @@ -602,7 +596,6 @@ static __attribute__((noreturn)) msg_t baseband_fn(void *arg) { chRegSetThreadName("baseband"); BasebandStatsCollector stats; - BasebandStatisticsMessage message; while(true) { // TODO: Place correct sampling rate into buffer returned here: @@ -616,11 +609,10 @@ static __attribute__((noreturn)) msg_t baseband_fn(void *arg) { } stats.process(buffer, - [&message](const BasebandStatistics statistics) { - if( message.is_free() ) { - message.statistics = statistics; - shared_memory.application_queue.push(&message); - } + [](const BasebandStatistics statistics) { + BasebandStatisticsMessage message; + message.statistics = statistics; + shared_memory.application_queue.push(message); } ); } @@ -632,7 +624,6 @@ static __attribute__((noreturn)) msg_t rssi_fn(void *arg) { chRegSetThreadName("rssi"); RSSIStatisticsCollector stats; - RSSIStatisticsMessage message; while(true) { // TODO: Place correct sampling rate into buffer returned here: @@ -643,11 +634,10 @@ static __attribute__((noreturn)) msg_t rssi_fn(void *arg) { stats.process( buffer, - [&message](const RSSIStatistics statistics) { - if( message.is_free() ) { - message.statistics = statistics; - shared_memory.application_queue.push(&message); - } + [](const RSSIStatistics statistics) { + RSSIStatisticsMessage message; + message.statistics = statistics; + shared_memory.application_queue.push(message); } ); } @@ -731,43 +721,40 @@ public: private: MessageHandlerMap message_map; - ChannelSpectrumMessage spectrum_message; - std::array spectrum_db; - void handle_baseband_queue() { while( !shared_memory.baseband_queue.is_empty() ) { - auto message = shared_memory.baseband_queue.pop(); - - message_map.send(message); - - message->state = Message::State::Free; + std::array message_buffer; + const Message* const message = reinterpret_cast(message_buffer.data()); + const auto message_size = shared_memory.baseband_queue.pop(message_buffer.data(), message_buffer.size()); + if( message_size ) { + message_map.send(message); + } } } void handle_spectrum() { if( channel_spectrum_request_update ) { /* Decimated buffer is full. Compute spectrum. */ - std::array, 256> samples_swapped; + std::array, channel_spectrum.size()> samples_swapped; fft_swap(channel_spectrum, samples_swapped); channel_spectrum_request_update = false; fft_c_preswapped(samples_swapped); - if( spectrum_message.is_free() ) { - for(size_t i=0; i .magnitude, or something more (less!) accurate. */ - spectrum_message.spectrum.db = &spectrum_db; - spectrum_message.spectrum.db_count = spectrum_db.size(); - spectrum_message.spectrum.sampling_rate = channel_spectrum_sampling_rate; - spectrum_message.spectrum.channel_filter_pass_frequency = channel_filter_pass_frequency; - spectrum_message.spectrum.channel_filter_stop_frequency = channel_filter_stop_frequency; - shared_memory.application_queue.push(&spectrum_message); + ChannelSpectrumMessage spectrum_message; + for(size_t i=0; i .magnitude, or something more (less!) accurate. */ + spectrum_message.spectrum.db_count = spectrum_message.spectrum.db.size(); + spectrum_message.spectrum.sampling_rate = channel_spectrum_sampling_rate; + spectrum_message.spectrum.channel_filter_pass_frequency = channel_filter_pass_frequency; + spectrum_message.spectrum.channel_filter_stop_frequency = channel_filter_stop_frequency; + shared_memory.application_queue.push(spectrum_message); } } }; diff --git a/firmware/common/message.hpp b/firmware/common/message.hpp index 5ab4fccb..72f73369 100644 --- a/firmware/common/message.hpp +++ b/firmware/common/message.hpp @@ -31,7 +31,9 @@ class Message { public: - enum class ID : uint16_t { + static constexpr size_t MAX_SIZE = 276; + + enum class ID : uint32_t { /* Assign consecutive IDs. IDs are used to index array. */ RSSIStatistics = 0, BasebandStatistics = 1, @@ -47,22 +49,11 @@ public: constexpr Message( ID id - ) : id { id }, - state { State::Free } + ) : id { id } { } - enum class State : uint16_t { - Free, - InUse, - }; - - bool is_free() const { - return state == State::Free; - } - const ID id; - volatile State state; }; struct RSSIStatistics { @@ -183,7 +174,7 @@ public: }; struct ChannelSpectrum { - std::array* db { nullptr }; + std::array db { { 0 } }; size_t db_count { 256 }; uint32_t sampling_rate { 0 }; uint32_t channel_filter_pass_frequency { 0 }; @@ -255,9 +246,11 @@ public: } void send(const Message* const message) { - auto& fn = map_[toUType(message->id)]; - if( fn ) { - fn(message); + if( message->id < Message::ID::MAX ) { + auto& fn = map_[toUType(message->id)]; + if( fn ) { + fn(message); + } } } diff --git a/firmware/common/message_queue.cpp b/firmware/common/message_queue.cpp index f443d25f..40705a93 100644 --- a/firmware/common/message_queue.cpp +++ b/firmware/common/message_queue.cpp @@ -26,56 +26,26 @@ using namespace lpc43xx; -bool MessageQueue::push(Message* const message) { - /* Returns true if success: - * - Message not in use. - * - FIFO wasn't full. - */ - if( message->state == Message::State::Free ) { - message->state = Message::State::InUse; - - if( enqueue(message) ) { - signal(); - return true; - } else { - // Roll back message state. - message->state = Message::State::Free; - } +bool MessageQueue::push(const void* const buf, const size_t len) { + const auto result = fifo.in_r(buf, len); + const bool success = (result == len); + if( success ) { + signal(); } - - return false; + return success; } -Message* MessageQueue::pop() { - /* TODO: Because of architecture characteristics, the two LSBs of the - * message pointer will always be 0. Other (non-pointer) message types - * could be encoded by setting these two bits to non-zero values. - * One of the bits could also be used as an "ack" flag... In fact, a - * pointer message could be turned into an "ack" message, or something - * like that... - * Might be better though to use formal operating structures in the - * message to do synchronization between processors. - */ - Message* message { nullptr }; - const auto success = fifo.out(&message, 1); - return success ? message : nullptr; +size_t MessageQueue::pop(void* const buf, const size_t len) { + return fifo.out_r(buf, len); } #if defined(LPC43XX_M0) -bool MessageQueue::enqueue(Message* const message) { - return fifo.in(&message, 1); -} - void MessageQueue::signal() { creg::m0apptxevent::assert(); } #endif #if defined(LPC43XX_M4) -bool MessageQueue::enqueue(Message* const message) { - return fifo.in(&message, 1); -} - void MessageQueue::signal() { creg::m4txevent::assert(); } diff --git a/firmware/common/message_queue.hpp b/firmware/common/message_queue.hpp index 704ddc19..400ecdea 100644 --- a/firmware/common/message_queue.hpp +++ b/firmware/common/message_queue.hpp @@ -29,9 +29,15 @@ class MessageQueue { public: - bool push(Message* const message); + template + bool push(const T& message) { + static_assert(sizeof(T) <= Message::MAX_SIZE, "Message::MAX_SIZE too small for message type"); + static_assert(std::is_base_of::value, "type is not based on Message"); - Message* pop(); + return push(&message, sizeof(message)); + } + + size_t pop(void* const buf, const size_t len); size_t len() const { return fifo.len(); @@ -42,9 +48,10 @@ public: } private: - FIFO fifo; + FIFO fifo; + + bool push(const void* const buf, const size_t len); - bool enqueue(Message* const message); void signal(); };