Feature/app slam#233
Conversation
| /** | ||
| * @file | ||
| * @brief SLAM application | ||
| * @author Gabryel Reyes <gabryelrdiaz@gmail.com> |
| /** | ||
| * @file | ||
| * @brief SLAM application | ||
| * @author Gabryel Reyes <gabryelrdiaz@gmail.com> |
| /** | ||
| * @file | ||
| * @brief Channel structure definition for the SerialMuxProt. | ||
| * @author Gabryel Reyes <gabryelrdiaz@gmail.com> |
There was a problem hiding this comment.
Replace (although file content is yet a copy)
| * Flag for setting initial data through SMP. | ||
| */ |
There was a problem hiding this comment.
Use clang-format (indent is wrong)
| String m_tcpRxBuffer; | ||
|
|
||
| /** Last reconnect attempt time. */ | ||
| uint32_t m_lastReconnectAttempt; |
There was a problem hiding this comment.
uint32_t m_lastReconnectAttemptTimeMs
| static LogSinkPrinter gLogSinkSerial("Serial", &Serial); | ||
|
|
||
| /** TCP reconnect interval in milliseconds. */ | ||
| static const uint32_t TCP_RECONNECT_INTERVAL = 2000U; |
There was a problem hiding this comment.
| static const uint32_t TCP_RECONNECT_INTERVAL = 2000U; | |
| static const uint32_t TCP_RECONNECT_INTERVAL_MS = 2000U; |
| bool setupSerialMuxProtServer(); | ||
|
|
||
| /** | ||
| * Connect to the TCP server. |
There was a problem hiding this comment.
What is this TCP server about? Describe either here or in the brief of class App itself.
This is needed to help understand the approach (and if it's valid).
| * | ||
| * @returns true if successful, otherwise false. | ||
| */ | ||
| bool connectToServer(); |
There was a problem hiding this comment.
Would be helpful if "Server" could get a bit more detailed.
| bool connectToServer(); | ||
|
|
||
| /** | ||
| * Send a TCP packet. |
There was a problem hiding this comment.
To where? Make sure that comments add value.
| void sendPacket(const String& payload); | ||
|
|
||
| /** | ||
| * Receive TCP packets. |
There was a problem hiding this comment.
From where? Make sure that comments add value. What happens with the received packets?
| const int newlineIndex = m_tcpRxBuffer.lastIndexOf(String("\n")); | ||
|
|
||
| if (newlineIndex >= 0) | ||
| { | ||
| m_tcpRxBuffer = m_tcpRxBuffer.substring(0U, static_cast<unsigned int>(newlineIndex)); | ||
| } |
There was a problem hiding this comment.
This cannot happen since you made sure that m_tcpRxBuffer += c; is only performed if not '\n'.
| } | ||
| else | ||
| { | ||
| m_tcpRxBuffer += c; |
There was a problem hiding this comment.
Add some kind of bounding here, like:
if (m_tcpRxBuffer.length() >= MAX_LINE_LEN)
{
LOG_WARNING("TCP line too long, discarding.");
m_tcpRxBuffer = String();
}
| if (true == m_tcpClient.connected()) | ||
| { | ||
| String packet = payload + '\n'; | ||
| (void)m_tcpClient.write(reinterpret_cast<const uint8_t*>(packet.c_str()), packet.length()); |
There was a problem hiding this comment.
Do not swallow the return value
|
|
||
| if (DeserializationError::Ok == err) | ||
| { | ||
| if ((!doc["linear"].isNull()) && (!doc["angular"].isNull())) |
There was a problem hiding this comment.
Lets use constants for "linear" and "angular" here and below.
| } | ||
| else | ||
| { | ||
| LOG_DEBUG("Velocity command sent - Linear: %d mm/s, Angular: %d mrad/s", linearCenter, angular); |
There was a problem hiding this comment.
No need to repeat "Linear: %d mm/s, Angular: %d mrad/s" here since the INFO logging a few lines above already made sure we got this.
2 participants
No description provided.