Comments (11)
Those look like good suggestions.
from min.
I would tell you that the most important is the compilation bug, can you fix it ? Else I would need to fork your project...
About the suggestion, I can live without but I think they will bring more flexibility to the API.
How much time do you plan to do the fix and API ? Or do you want me to make a PR for
- Compilation bug
- Suggestion 1
For suggestion 2 I'm now sure if there something else to do except the fix I proposed.
from min.
Done all three fixes: 4626d73
The compilation option is to suspend periodic ACK retransmission rather than include it so that the default is to include it as at present.
from min.
Wow, thank your so much @kentindell.
Last thing
Suggestion 3
What do you think to let us use a lookup table to compute CRC32 instead of expensive nested for instruction (bytes + bits) ? If size is an issue on embedded side, we can use a macro to indicate that we don't want to use table and fallback to your old method ? Something like this
#ifdef MIN_TRANSPORT_PROTOCOL
static void crc32_init_context(struct crc32_context *context)
{
context->crc = 0xffffffffU;
}
#ifdef MIN_TRANSPORT_USE_CRC_TABLE
static uint32_t CRC32TableSize = 256;
static uint32_t CRC32Table[CRCTableSize] =
{
0x00000000, 0x77073096, 0xee0e612c, 0x990951ba,
0x076dc419, 0x706af48f, 0xe963a535, 0x9e6495a3,
0x0edb8832, 0x79dcb8a4, 0xe0d5e91e, 0x97d2d988,
0x09b64c2b, 0x7eb17cbd, 0xe7b82d07, 0x90bf1d91,
0x1db71064, 0x6ab020f2, 0xf3b97148, 0x84be41de,
0x1adad47d, 0x6ddde4eb, 0xf4d4b551, 0x83d385c7,
0x136c9856, 0x646ba8c0, 0xfd62f97a, 0x8a65c9ec,
0x14015c4f, 0x63066cd9, 0xfa0f3d63, 0x8d080df5,
0x3b6e20c8, 0x4c69105e, 0xd56041e4, 0xa2677172,
0x3c03e4d1, 0x4b04d447, 0xd20d85fd, 0xa50ab56b,
0x35b5a8fa, 0x42b2986c, 0xdbbbc9d6, 0xacbcf940,
0x32d86ce3, 0x45df5c75, 0xdcd60dcf, 0xabd13d59,
0x26d930ac, 0x51de003a, 0xc8d75180, 0xbfd06116,
0x21b4f4b5, 0x56b3c423, 0xcfba9599, 0xb8bda50f,
0x2802b89e, 0x5f058808, 0xc60cd9b2, 0xb10be924,
0x2f6f7c87, 0x58684c11, 0xc1611dab, 0xb6662d3d,
0x76dc4190, 0x01db7106, 0x98d220bc, 0xefd5102a,
0x71b18589, 0x06b6b51f, 0x9fbfe4a5, 0xe8b8d433,
0x7807c9a2, 0x0f00f934, 0x9609a88e, 0xe10e9818,
0x7f6a0dbb, 0x086d3d2d, 0x91646c97, 0xe6635c01,
0x6b6b51f4, 0x1c6c6162, 0x856530d8, 0xf262004e,
0x6c0695ed, 0x1b01a57b, 0x8208f4c1, 0xf50fc457,
0x65b0d9c6, 0x12b7e950, 0x8bbeb8ea, 0xfcb9887c,
0x62dd1ddf, 0x15da2d49, 0x8cd37cf3, 0xfbd44c65,
0x4db26158, 0x3ab551ce, 0xa3bc0074, 0xd4bb30e2,
0x4adfa541, 0x3dd895d7, 0xa4d1c46d, 0xd3d6f4fb,
0x4369e96a, 0x346ed9fc, 0xad678846, 0xda60b8d0,
0x44042d73, 0x33031de5, 0xaa0a4c5f, 0xdd0d7cc9,
0x5005713c, 0x270241aa, 0xbe0b1010, 0xc90c2086,
0x5768b525, 0x206f85b3, 0xb966d409, 0xce61e49f,
0x5edef90e, 0x29d9c998, 0xb0d09822, 0xc7d7a8b4,
0x59b33d17, 0x2eb40d81, 0xb7bd5c3b, 0xc0ba6cad,
0xedb88320, 0x9abfb3b6, 0x03b6e20c, 0x74b1d29a,
0xead54739, 0x9dd277af, 0x04db2615, 0x73dc1683,
0xe3630b12, 0x94643b84, 0x0d6d6a3e, 0x7a6a5aa8,
0xe40ecf0b, 0x9309ff9d, 0x0a00ae27, 0x7d079eb1,
0xf00f9344, 0x8708a3d2, 0x1e01f268, 0x6906c2fe,
0xf762575d, 0x806567cb, 0x196c3671, 0x6e6b06e7,
0xfed41b76, 0x89d32be0, 0x10da7a5a, 0x67dd4acc,
0xf9b9df6f, 0x8ebeeff9, 0x17b7be43, 0x60b08ed5,
0xd6d6a3e8, 0xa1d1937e, 0x38d8c2c4, 0x4fdff252,
0xd1bb67f1, 0xa6bc5767, 0x3fb506dd, 0x48b2364b,
0xd80d2bda, 0xaf0a1b4c, 0x36034af6, 0x41047a60,
0xdf60efc3, 0xa867df55, 0x316e8eef, 0x4669be79,
0xcb61b38c, 0xbc66831a, 0x256fd2a0, 0x5268e236,
0xcc0c7795, 0xbb0b4703, 0x220216b9, 0x5505262f,
0xc5ba3bbe, 0xb2bd0b28, 0x2bb45a92, 0x5cb36a04,
0xc2d7ffa7, 0xb5d0cf31, 0x2cd99e8b, 0x5bdeae1d,
0x9b64c2b0, 0xec63f226, 0x756aa39c, 0x026d930a,
0x9c0906a9, 0xeb0e363f, 0x72076785, 0x05005713,
0x95bf4a82, 0xe2b87a14, 0x7bb12bae, 0x0cb61b38,
0x92d28e9b, 0xe5d5be0d, 0x7cdcefb7, 0x0bdbdf21,
0x86d3d2d4, 0xf1d4e242, 0x68ddb3f8, 0x1fda836e,
0x81be16cd, 0xf6b9265b, 0x6fb077e1, 0x18b74777,
0x88085ae6, 0xff0f6a70, 0x66063bca, 0x11010b5c,
0x8f659eff, 0xf862ae69, 0x616bffd3, 0x166ccf45,
0xa00ae278, 0xd70dd2ee, 0x4e048354, 0x3903b3c2,
0xa7672661, 0xd06016f7, 0x4969474d, 0x3e6e77db,
0xaed16a4a, 0xd9d65adc, 0x40df0b66, 0x37d83bf0,
0xa9bcae53, 0xdebb9ec5, 0x47b2cf7f, 0x30b5ffe9,
0xbdbdf21c, 0xcabac28a, 0x53b39330, 0x24b4a3a6,
0xbad03605, 0xcdd70693, 0x54de5729, 0x23d967bf,
0xb3667a2e, 0xc4614ab8, 0x5d681b02, 0x2a6f2b94,
0xb40bbe37, 0xc30c8ea1, 0x5a05df1b, 0x2d02ef8d
};
static void crc32_step(struct crc32_context *context, uint8_t byte)
{
context->crc = (context->crc >> 8) ^ CRC32Table[(context->crc ^ (uint32_t)byte) & 0x000000FFul];
}
static uint32_t crc32_finalize(struct crc32_context *context)
{
return context->crc;
}
#elif // MIN_TRANSPORT_USE_CRC_TABLE
static void crc32_step(struct crc32_context *context, uint8_t byte)
{
context->crc ^= byte;
for(uint32_t j = 0; j < 8; j++) {
uint32_t mask = (uint32_t) -(context->crc & 1U);
context->crc = (context->crc >> 1) ^ (0xedb88320U & mask);
}
}
static uint32_t crc32_finalize(struct crc32_context *context)
{
return ~context->crc;
}
#endif // MIN_TRANSPORT_USE_CRC_TABLE
#endif // MIN_TRANSPORT_PROTOCOL
from min.
I'm going to revisit the CRC issue completely after discovering the research that Koopman did at CMU. I had originally used Fletcher's Checksum, which was very cheap to compute. But I also need to put an individual CRC for just the header (8-bit CRC should be OK) and then a CRC for the rest of the payload. But the CRC polynomial should be picked for a particular Hamming distance requirement: not all polynomials are the same.
As well as the above, CRC hardware is becoming quite common in MCUs so the software should allow the CRC implementation to be "plugged", which would allow CRC hardware to be driven, or a lookup table implementation, or a CPU-intensive one, all depending on the requirements of the target device. I do want MIN to be workable for small AVR or PIC devices, which don't have the space for a lookup table or have CRC hardware. So the 'driver API' model is appropriate for this.
from min.
I'm going to revisit the CRC issue completely after discovering
Great, anyway this is not a urgent concern for me....
Suggestion 4
As you probably guest, I'm using the target
source code version for Visual Studio 2015 compiler and GCC 4.9. I need to use it on small linux single board and also on demo application running on Windows Desktop. I don't want to use Pyton
for desktop version and converting all your works to C++ would be my last options.
So... to be able to use min-protocol in a C++ environment, it would be more flexible to pass a void *ext
instead of a uint8_t port
. This way we could use the ext pointer to jump to a c++ class member and on a specific instance. Ex :
#include <cstdint>
#include <vector>
extern "C"
{
#include <min.h>
}
class MySerialPort
{
public:
void doSomething();
void doSomethingMore();
void init()
{
min_init_context(&ctx, static_cast<void *>(this));
}
min_context ctx;
std::vector<uint8_t> txBuffer;
}
void min_tx_start(void* ext)
{
/* ext will have been passed to min_init_context and pass back to all min callback
method to allow dereferencing a MySerialPort instance. Otherwise I would have
to declare as much buffer as I will have possible instance of serial
port (MySerialPort). */
MySerialPort* my = static_cast<MySerialPort *>(ext);
my.txBuffer.clear();
}
Question
I'm not sure to understand why you test the value of remoteConnected
on this line https://github.com/min-protocol/min/blob/master/target/min.c#L588. Why not keep sending the the last frame for ever or until we get a reset ? By the way, I create a server/client test suite and your protocol implementation rock, I've test a lot of synchronization potential problem and each time the synchronization come back.
from min.
I'll look at making that change to support C++: it's a good idea.
To answer the question, I did used to just resend the data but it screws up some systems that buffer the data at the receiver end even if the app consuming the data has stopped (I had trouble with Windows serial and USB).
from min.
Thank you for your answer. For now I'm re-implementing a C++ version as I need it right now.
Bug ?
Here https://github.com/min-protocol/min/blob/master/target/min.c#L152, we should not read
assert(elf->transport_fifo.n_frames != 0);
instead of
assert(n_frames != 0);
?
from min.
Yeah, looks like a bug. Weird: that wouldn't even compile yet I'm pretty sure the Arduino build testing was done with assertion checking turned on.
from min.
Sorry to disturb you again.
Compilation Error
static void crc32_step(struct crc32_context *context, uint8_t byte)
{
context->crc ^= byte;
for(uint32_t j = 0; j < 8; j++) {
uint32_t mask = (uint32_t) -(context->crc & 1U); <-- THIS LINE
context->crc = (context->crc >> 1) ^ (0xedb88320U & mask);
}
}
On Visual Studio 2015, I'm getting this compilation error
C4146 unary minus operator applied to unsigned type, result still unsigned
Which could be fix by
static void crc32_step(struct crc32_context *context, uint8_t byte)
{
context->crc ^= byte;
for(uint32_t j = 0; j < 8; j++) {
context->crc = (context->crc >> 1) ^ (context->crc & 1)* 0xedb88320U;
}
}
from min.
I don't think that's a C error, it's just a weird aspect of the language worthy of a warning. It does compile efficiently on a small MCU to a simple 2's complement operation.
But in any case I am going to revisit all the CRC code anyway: I don't think 32 bits is required and a good polynomial for a 24-bit CRC should work just as well (maybe the one FlexRay uses).
from min.
Related Issues (20)
- MIN protocol not working properly on TI MCUs but works on STM HOT 2
- Escape 0x03 (CTRL-C)
- Question regarding the signal layer json HOT 2
- Question: Is Data-Streaming Supported? HOT 2
- Ability to calculate CRC in HW HOT 1
- Time-based frame-delimiting HOT 4
- A Rust library for MIN. HOT 1
- Any port for C#? HOT 2
- STM32Cube USB VCP HOT 9
- Review Comments
- Received spurious ACK HOT 2
- Testing MIN with serial monitor HOT 2
- How to include in C++ project? HOT 1
- Possible bug in the code
- Multiple Ports HOT 2
- Memory conflict: Multiple MIN ports access the same payloads_ring_buffer HOT 2
- Time variable not initialized in transport_fifo_reset HOT 1
- Python package
- What is the best practice for calling min_poll HOT 4
- Python host vs target C implementations
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from min.