proposal for I2C master/slave example #5360
Conversation
…precated and two begins are ambiguous
| uint16_t _xorIn; | ||
| uint16_t _xorOut; | ||
| uint16_t _polynomial; | ||
| uint8_t _reflectIn; |
There was a problem hiding this comment.
I guess these are uint8_t instead of bool for size? It's ok to do that, but at the very least the interface args should be bool type.
| _mask = 0xFFFF; | ||
| _crc = _xorIn; | ||
| } | ||
| inline Crc16(uint8_t reflectIn, uint8_t reflectOut, uint16_t polynomial, uint16_t xorIn, uint16_t xorOut, uint16_t msbMask, uint16_t mask) |
There was a problem hiding this comment.
I suggest the args reflectIn/Out be bool type
| inline void clearCrc(); | ||
| inline void updateCrc(uint8_t data); | ||
| inline uint16_t getCrc(); | ||
| inline unsigned int fastCrc(uint8_t data[], uint8_t start, uint16_t length, uint8_t reflectIn, uint8_t reflectOut, uint16_t polynomial, uint16_t xorIn, uint16_t xorOut, uint16_t msbMask, uint16_t mask); |
| inline void updateCrc(uint8_t data); | ||
| inline uint16_t getCrc(); | ||
| inline unsigned int fastCrc(uint8_t data[], uint8_t start, uint16_t length, uint8_t reflectIn, uint8_t reflectOut, uint16_t polynomial, uint16_t xorIn, uint16_t xorOut, uint16_t msbMask, uint16_t mask); | ||
| inline unsigned int XModemCrc(uint8_t data[], uint8_t start, uint16_t length) |
There was a problem hiding this comment.
I suggest changing the signature to:
template <std::size_t dataSize>
unsigned int XModemCrc(const uint8_t (data&)[dataSize], const uint8_t start)
This assures that the dataSize value always corresponds to the size of the array passed in as an arg.
Also, the user has one less arg to pass into the function.
And, because it's a 1-line forwarder, it gets optimized away.
However, this works only for arrays of predetermined size, and not for dynamically allocated arrays. In this last case, the function signature to use should be:
inline unsigned int XModemCrc(const uint8_t *data, const uint8_t start, const uint16_t length)
Of course, both can coexist as overloads.
| @@ -0,0 +1,209 @@ | |||
| //------------------------------------------------------------------------------------- | |||
There was a problem hiding this comment.
I know this is not your code, but I thought to add some suggestions anyways. It's ok to ignore them 😛
Globally, consider adding const-ness to all relevant args.
| } | ||
| Serial.print(address, HEX); | ||
| Serial.println(" !"); | ||
| nDevices++; |
| Serial.println(datasize); | ||
|
|
||
| // Generate new data set for the struct | ||
| for (size_t i = 0; i < datasize; i++) { |
|
|
||
| } | ||
|
|
||
| void receiveEvent(int howMany) { |
| bool requestRetransfer = false; | ||
|
|
||
| unsigned long rtimeout = millis() + 200; // skip 100ms timeouts | ||
| while (millis() < rtimeout) { |
There was a problem hiding this comment.
How would you solve this without getting involved in something complex? It's not that important, it might be removed instead.
I do understand, that the rtimeout can become a less than millis almost forever in certain moment. However it should take just another 200 millis until it will get unstuck, as real millis will reset back to zero soon as well.
There was a problem hiding this comment.
I think you should subtract current millis from the original value:
unsigned long rtimeout = millis();
while (millis() - rtimeout < 200) {
| } // if 0 | ||
|
|
||
| digitalWrite(LED_PIN, HIGH); | ||
| index++; |
|
Fixes #5288 |
|
Jsem debil, díky :o)
… On 24 Nov 2018, at 22:48, Jiri Bilek ***@***.***> wrote:
@JiriBilek commented on this pull request.
In libraries/Wire/examples/i2c-P2P-slave/i2c-P2P-slave.ino <#5360 (comment)>:
> +
+void receiveEvent(int howMany) {
+
+ Serial.print("Received "); Serial.print(howMany); Serial.println(" bytes...");
+ char c;
+ char chars[howMany];
+ chars[howMany - 1] = '\0';
+ int index = 0;
+
+ Serial.print("Local Sequence: "); Serial.println((int)seq);
+ digitalWrite(LED_PIN, LOW);
+
+ bool requestRetransfer = false;
+
+ unsigned long rtimeout = millis() + 200; // skip 100ms timeouts
+ while (millis() < rtimeout) {
I think you should subtract original millis with the current one:
unsigned long rtimeout = millis();
while (millis() - rtimeout < 200) {
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#5360 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AANix9UMCJeTB7js7H21n0Ztb4BRl57Iks5uyb66gaJpZM4Yt4sa>.
|
…o into i2c_master_slave_example
…o into i2c_master_slave_example
|
Closing in favor of #5464 . |
Please comment code style and propose way to share code. Yes, there are duplicities. However, this is example where code-bases for master and slave will be always expectably different and because Arduino IDE does not allow two sketches in one folder, only way would be to create a library or use CRC16 from core (if any) to share code in the example.