search

min-protocol / min

The MIN protocol specification and reference implementation
257 stars 88 forks source link

warning: comparison is always true #9

Closed senilix closed 7 years ago

senilix commented 7 years ago

Hi Ken,

First, thanks for what looks to be a great little protocol. However, during compilation I ran into following warning. I don't know how critical it is in itself, but it do prevent you code from doing a conditional decision as you intended.

min.c:497:42: warning: comparison is always true due to limited range of data type [-Wtype-limits]
                 if(self->rx_frame_length <= MAX_PAYLOAD) {

I run gcc compiler options -Wall -Wextra

Cheers, Mogens

kentindell commented 7 years ago

Yep, that's deliberate: I wanted to make sure that if the code is compiled with MAX_PAYLOAD set to be smaller than 256 (to save buffer space on a really tiny AVR or something) then too-big frames will be rejected.

I'll update the code to use the preprocessor to eliminate the test if MAX_PAYLOAD is 256. That will eliminate the warning.

senilix commented 7 years ago

Hi Ken,

Thanks. It's not a show stopper for me. Anyway, even though it's nice it's able to run on Arduino I think a light weight protocol that also supports larger packets than 256 bytes would be nice.

As a general note. How do you like to receive enhancement suggestions?

Mogens

kentindell commented 7 years ago

I have thought about even larger packets: using a 16-bit length and making it up to 65K. But if there's line noise then the performance becomes horrific. It's probably better to build a streaming/file transfer layer on top of the transport layer so that large blocks of data can be transmitted in sequential packets. It's pretty straightforward with the transport protocol already ensuring reliable in-order delivery.

For any enhancement suggestions it's best to raise an issue and: I'll tag it up and everyone can pitch in with comments.

senilix commented 7 years ago

Yes, that would also be a possibility.

I will close this issue and open a new with the question/suggestion for a change.