Skip to content

Commit

Permalink
Minor fixes to SNIP client and SNIP handler: (#600)
Browse files Browse the repository at this point in the history
- Adds proper shutdown to SNIP client. This is needed for unittesting purposes.
- Fixes crashing bug in SNIP client when returned payload is empty.
- Moves SNIP handler's static descriptor to a separate compilation unit to avoid
  it being unexpectedly linked into binaries. This is not only flash space wasted,
  but also presents an unexpected dependency on SNIP_DYNAMIC_FILENAME being available.
  • Loading branch information
balazsracz authored Jan 15, 2022
1 parent 80ed398 commit 9ed634f
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 19 deletions.
12 changes: 12 additions & 0 deletions src/openlcb/SNIPClient.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#include "executor/CallableFlow.hxx"
#include "openlcb/Defs.hxx"
#include "openlcb/If.hxx"
#include "os/sleep.h"

namespace openlcb
{
Expand Down Expand Up @@ -88,6 +89,17 @@ public:
{
}

/// Flushes the pending timed operations.
void shutdown()
{
while (!is_waiting())
{
service()->executor()->sync_run(
[this]() { timer_.ensure_triggered(); });
microsleep(500);
}
}

Action entry() override
{
request()->resultCode = SNIPClientRequest::OPERATION_PENDING;
Expand Down
24 changes: 5 additions & 19 deletions src/openlcb/SimpleNodeInfo.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -43,24 +43,6 @@ namespace openlcb
extern const SimpleNodeStaticValues __attribute__((weak)) SNIP_STATIC_DATA = {
4, "OpenMRN", "Undefined model", "Undefined HW version", "0.9"};

const SimpleInfoDescriptor SNIPHandler::SNIP_RESPONSE[] = {
{SimpleInfoDescriptor::LITERAL_BYTE, 4, 0, nullptr},
{SimpleInfoDescriptor::C_STRING, 0, 0, SNIP_STATIC_DATA.manufacturer_name},
{SimpleInfoDescriptor::C_STRING, 0, 0, SNIP_STATIC_DATA.model_name},
{SimpleInfoDescriptor::C_STRING, 0, 0, SNIP_STATIC_DATA.hardware_version},
{SimpleInfoDescriptor::C_STRING, 0, 0, SNIP_STATIC_DATA.software_version},
#if OPENMRN_HAVE_POSIX_FD
{SimpleInfoDescriptor::FILE_LITERAL_BYTE, 2, 0, SNIP_DYNAMIC_FILENAME},
{SimpleInfoDescriptor::FILE_C_STRING, 63, 1, SNIP_DYNAMIC_FILENAME},
{SimpleInfoDescriptor::FILE_C_STRING, 64, 64, SNIP_DYNAMIC_FILENAME},
#else
/// @todo(balazs.racz) Add eeprom support to arduino.
{SimpleInfoDescriptor::LITERAL_BYTE, 2, 0, nullptr},
{SimpleInfoDescriptor::LITERAL_BYTE, 0, 0, nullptr},
{SimpleInfoDescriptor::LITERAL_BYTE, 0, 0, nullptr},
#endif
{SimpleInfoDescriptor::END_OF_DATA, 0, 0, 0}};

void init_snip_user_file(int fd, const char *user_name,
const char *user_description)
{
Expand All @@ -86,7 +68,7 @@ void init_snip_user_file(int fd, const char *user_name,
}

static size_t find_string_at(const openlcb::Payload& payload, size_t start_pos, string* output) {
if (start_pos == string::npos) {
if (start_pos >= payload.size()) {
output->clear();
return start_pos;
}
Expand All @@ -103,6 +85,10 @@ void decode_snip_response(
const openlcb::Payload &payload, SnipDecodedData *output)
{
output->clear();
if (payload.empty())
{
return;
}
char sys_ver = payload[0];
size_t pos = 1;
pos = find_string_at(payload, pos, &output->manufacturer_name);
Expand Down
58 changes: 58 additions & 0 deletions src/openlcb/SimpleNodeInfoResponse.cxx
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/** \copyright
* Copyright (c) 2022, Balazs Racz
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
*
* - Redistributions of source code must retain the above copyright notice,
* this list of conditions and the following disclaimer.
*
* - Redistributions in binary form must reproduce the above copyright notice,
* this list of conditions and the following disclaimer in the documentation
* and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGE.
*
* \file SimpleNodeInfoResponse.cxx
*
* Response descriptor for the Simple Node Ident Info protocol handler.
*
* @author Balazs Racz
* @date 3 Jan 2022
*/

#include "openlcb/SimpleNodeInfo.hxx"

namespace openlcb
{

const SimpleInfoDescriptor SNIPHandler::SNIP_RESPONSE[] = {
{SimpleInfoDescriptor::LITERAL_BYTE, 4, 0, nullptr},
{SimpleInfoDescriptor::C_STRING, 0, 0, SNIP_STATIC_DATA.manufacturer_name},
{SimpleInfoDescriptor::C_STRING, 0, 0, SNIP_STATIC_DATA.model_name},
{SimpleInfoDescriptor::C_STRING, 0, 0, SNIP_STATIC_DATA.hardware_version},
{SimpleInfoDescriptor::C_STRING, 0, 0, SNIP_STATIC_DATA.software_version},
#if OPENMRN_HAVE_POSIX_FD
{SimpleInfoDescriptor::FILE_LITERAL_BYTE, 2, 0, SNIP_DYNAMIC_FILENAME},
{SimpleInfoDescriptor::FILE_C_STRING, 63, 1, SNIP_DYNAMIC_FILENAME},
{SimpleInfoDescriptor::FILE_C_STRING, 64, 64, SNIP_DYNAMIC_FILENAME},
#else
/// @todo(balazs.racz) Add eeprom support to arduino.
{SimpleInfoDescriptor::LITERAL_BYTE, 2, 0, nullptr},
{SimpleInfoDescriptor::LITERAL_BYTE, 0, 0, nullptr},
{SimpleInfoDescriptor::LITERAL_BYTE, 0, 0, nullptr},
#endif
{SimpleInfoDescriptor::END_OF_DATA, 0, 0, 0}};

}
1 change: 1 addition & 0 deletions src/openlcb/sources
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ CXXSRCS += \
DatagramTcp.cxx \
MemoryConfig.cxx \
SimpleNodeInfo.cxx \
SimpleNodeInfoResponse.cxx \
SimpleNodeInfoMockUserFile.cxx \
SimpleStack.cxx \
TractionTestTrain.cxx \
Expand Down

0 comments on commit 9ed634f

Please sign in to comment.