Skip to content

Commit

Permalink
Applied review feedback
Browse files Browse the repository at this point in the history 
#185
  • Loading branch information
@nhjschulz
nhjschulz committed Oct 31, 2024
1 parent ab1f873 commit 839938f
Show file tree
Hide file tree
Showing 9 changed files with 230 additions and 119 deletions.
32 changes: 32 additions & 0 deletions config/buildmode.ini
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
; ********************************************************************************
; Build mode log configurations
; * debug: Standard debugging purposes during development.
; * release: The release build contains only the relevant user informations.
; * trace: Hardcore debugging by following e.g. method call hierarchy.
; ********************************************************************************
[mode:debug_log]
build_flags =
-D LOG_DEBUG_ENABLE=1
-D LOG_TRACE_ENABLE=0
-D CONFIG_LOG_SEVERITY=Logging::LOG_LEVEL_DEBUG

[mode:release_log]
build_flags =
-D LOG_DEBUG_ENABLE=0
-D LOG_TRACE_ENABLE=0
-D CONFIG_LOG_SEVERITY=Logging::LOG_LEVEL_INFO

[mode:trace_log]
build_flags =
-D LOG_DEBUG_ENABLE=1
-D LOG_TRACE_ENABLE=1
-D CONFIG_LOG_SEVERITY=Logging::LOG_LEVEL_TRACE

[mode:debug_native]
build_flags =
-O0
-ggdb

[mode:release_native]
build_flags =
-O2
4 changes: 3 additions & 1 deletion doc/ROS2/setup/Agent.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ Start the MicroXRCEAgent binary to listen to **TCP** connections:
## Using the agent with the UDP interface

> [!WARNING]
> Note: UDP ports on WSL are not working properly if you need to access them outside of the WSL VM. Use TCP with WSL instead.
> Note: UDP ports on WSL are not working properly if you need to access them outside of the WSL VM (Status 2024-10-31). Use TCP with WSL instead.
> See [https://github.com/micro-ROS/micro-ROS-Agent/issues/194](https://github.com/micro-ROS/micro-ROS-Agent/issues/194) for further details.
> The mentioned ```netsh`` tool for port proxy forwarding only supports TCP.
Start the MicroXRCEAgent binary to listen to UDP connections:

Expand Down
6 changes: 3 additions & 3 deletions lib/APPTurtle/src/Transports/CustomRosTransportBase.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class CustomRosTransportBase
* Destroys custom Micro-ROS transport.
*
*/
~CustomRosTransportBase()
virtual ~CustomRosTransportBase()
{
}

Expand Down Expand Up @@ -166,8 +166,8 @@ class CustomRosTransportBase
static size_t read(uxrCustomTransport* transport, uint8_t* buffer, size_t size, int timeout, uint8_t* errorCode);

/**
* Get protocol name used by this trandport.
* @return protocol name
* Get protocol name used by this transport.
* @return Protocol name
*/
virtual const String& getProtocolName() const = 0;

Expand Down
80 changes: 48 additions & 32 deletions lib/APPTurtle/src/Transports/CustomRosTransportTcp.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,10 @@

const String CustomRosTransportTcp::m_protocolName("TCP");

const CustomRosTransportTcp::ReadFunc CustomRosTransportTcp::m_readFunction[MAX] =
const CustomRosTransportTcp::ReadFunc
CustomRosTransportTcp::m_readFunction[InputState::INPUT_STATE_MAX] =
{
&CustomRosTransportTcp::readSizePrefix,
&CustomRosTransportTcp::readSizePrefix,
&CustomRosTransportTcp::readPendingSizePrefix,
&CustomRosTransportTcp::readPayload,
&CustomRosTransportTcp::readFinish
Expand All @@ -84,7 +85,7 @@ bool CustomRosTransportTcp::open(void)
{
bool isOpen = false;

if (!m_tcpClient.connect(m_address, m_port))
if (0U == m_tcpClient.connect(m_address, m_port))
{
LOG_ERROR("TCP connect error");
}
Expand Down Expand Up @@ -113,22 +114,22 @@ size_t CustomRosTransportTcp::write(const uint8_t* buffer, size_t size, uint8_t*
else
{
uint8_t lengthPrefix[2];
lengthPrefix[0] = static_cast<uint8_t>(size & 0xFF);
lengthPrefix[1] = static_cast<uint8_t>((size >> 8) & 0xFF);
lengthPrefix[0] = static_cast<uint8_t>(size & 0xFFU);
lengthPrefix[1] = static_cast<uint8_t>((size >> 8U) & 0xFFU);

if (sizeof(lengthPrefix) != m_tcpClient.write(lengthPrefix, sizeof(lengthPrefix)))
{
LOG_ERROR("Write prefix error (size=%zu, sent=%zu)", size, sent);
*errorCode = 1;
*errorCode = 1U;
}
else if ((sent = m_tcpClient.write(buffer, size)) != size)
{
LOG_ERROR("Write data error (size=%zu, sent=%zu)", size, sent);
*errorCode = 1;
*errorCode = 1U;
}
else
{
*errorCode = 0;
*errorCode = 0U;
}
}

Expand All @@ -148,30 +149,45 @@ size_t CustomRosTransportTcp::read(uint8_t* buffer, size_t size, int timeout, ui
bool loop = true;

/**
* Run receive state machine.
* Run receive state machine.
* The read function relationships are:
*
*
* read() -> [state]->read() -> readInternal() -> WifiClient
*/
while (loop)
while (true == loop)
{
loop = (this->*m_readFunction[m_inputState])(timeout, errorCode);

if (InputState::INPUT_STATE_MAX > m_inputState)
{
ReadFunc stateReadFunc = m_readFunction[m_inputState]; /**< Get state dependend input reader. */

/* Read state dependend message portion. */
loop = (this->*stateReadFunc)(timeout, errorCode);
}
else
{
LOG_FATAL("Invalid state %d", static_cast<int>(m_inputState));
*errorCode = 4U;
loop = false;
}
}

if (InputState::FINISH == m_inputState)
if (InputState::INPUT_STATE_FINISH == m_inputState)
{
if (readBytes > size)
{
/* internal error, request buffer would be overrun */
/* Internal error, request buffer would be overrun. */
close();
* errorCode = 3U;
*errorCode = 5U;
}
else
else
{
/* Provide payload to destination buffer. */
readBytes = m_payloadLen;
memcpy(buffer, m_inputBuf, readBytes);

m_inputState = InputState::INIT;
/* Reset state machine*/
m_inputState = InputState::INPUT_STATE_INIT;
m_received = 0U;
m_payloadLen = 0U;
}
Expand All @@ -192,7 +208,7 @@ size_t CustomRosTransportTcp::readInternal(uint8_t* buffer, size_t size, int tim

readTimer.start(timeout);

while ((0 < remaining) && (false == timeOver))
while ((0U < remaining) && (false == timeOver))
{
int count = m_tcpClient.read(buffer, remaining);
if (-1 == count)
Expand Down Expand Up @@ -232,29 +248,29 @@ bool CustomRosTransportTcp::readSizePrefix(int timeout, uint8_t* errorCode)

switch (readInternal(prefix, sizeof(prefix), timeout, errorCode))
{
case 0: /* nothing received */
if (0 != *errorCode)
case 0U: /* nothing received */
if (0U != *errorCode)
{
close();
}
loop = false;
break;

case 1: /* only low byte out of the 2 length prefix bytes received. */
case 1U: /* only low byte out of the 2 length prefix bytes received. */
m_payloadLen = static_cast<size_t>(prefix[0]);
m_inputState = InputState::PREFIX_1;
m_inputState = InputState::INPUT_STATE_PREFIX_1;
loop = true;
break;

case 2: /* 2 byte prefix received */
case 2U: /* 2 byte prefix received */
m_payloadLen = static_cast<size_t>(prefix[0]) + (static_cast<size_t>(prefix[1]) << 8);
m_received = 0U;
m_inputState = InputState::PLAY_LOAD;
m_inputState = InputState::INPUT_STATE_PLAYLOAD;
loop = true;
break;

default: /* should never be possible but ...*/
*errorCode = 2;
*errorCode = 2U;
loop = false;
break;
}
Expand All @@ -264,28 +280,28 @@ bool CustomRosTransportTcp::readSizePrefix(int timeout, uint8_t* errorCode)

bool CustomRosTransportTcp::readPendingSizePrefix(int timeout, uint8_t* errorCode)
{
bool loop = true;
bool loop = true;
uint8_t prefix[1];

switch (readInternal(prefix, sizeof(prefix), timeout, errorCode))
{
case 0: /* nothing received */
case 0U: /* nothing received */
if (0U != *errorCode)
{
close();
}
loop = false;
break;

case 1: /* High byte received. */
case 1U: /* High byte received. */
m_payloadLen |= (static_cast<size_t>(prefix[0]) << 8);
m_received = 0U;
m_inputState = InputState::PLAY_LOAD;
m_inputState = InputState::INPUT_STATE_PLAYLOAD;
loop = true;
break;

default: /* should never be possible but ...*/
*errorCode = 2;
*errorCode = 2U;
loop = false;
break;
}
Expand All @@ -295,7 +311,7 @@ bool CustomRosTransportTcp::readPendingSizePrefix(int timeout, uint8_t* errorCod

bool CustomRosTransportTcp::readPayload(int timeout, uint8_t* errorCode)
{
bool loop = true;
bool loop = true;
size_t readByteCnt = readInternal(m_inputBuf + m_received, m_payloadLen - m_received, timeout, errorCode);

if (0U < readByteCnt)
Expand All @@ -304,7 +320,7 @@ bool CustomRosTransportTcp::readPayload(int timeout, uint8_t* errorCode)
if (m_received == m_payloadLen)
{
/* record completed */
m_inputState = InputState::FINISH;
m_inputState = InputState::INPUT_STATE_FINISH;
}
}
else
Expand Down
Loading

0 comments on commit 839938f

Please sign in to comment.