Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

indication() callback not called for all message types with RFM69 transport #584

Closed
janchrillesen opened this Issue Sep 19, 2016 · 9 comments

Comments

Projects
None yet
3 participants
@janchrillesen
Copy link
Contributor

janchrillesen commented Sep 19, 2016

(Follow up on this post - https://forum.mysensors.org/topic/4869/esp8266-gateway-webserver-awesome/18)

I have an ESP8266/RFM69 based gateway. I have added the following code:

void indication( const indication_t ind )
{
Serial.println("Indication callback");
Serial.print("Type: ");
Serial.println(ind);
}

When a client connects to the gateway I see a type 11 callback (INDICATION_PRESENT)
However, when the gateway receives (TSF:MSG:READ) and transmit I don't see any callbacks.

I have not tried to run the same code on my NRF24 based gateway, as it's inside a case and doesn't have easy access to the serial port, but as the webserver feature works, I assume it's related to RFM69 transport.

MySensors 2.0.1 beta (as of a week ago)

@tekka007

This comment has been minimized.

Copy link
Contributor

tekka007 commented Sep 19, 2016

I suspect a buffer overflow. Can you substitute

memcpy(data,(const void *)_radio.DATA, _radio.DATALEN);
to

memcpy(data,(const void *)_radio.DATA, min(MAX_PAYLOAD,_radio.DATALEN));
in

https://github.com/mysensors/MySensors/blob/development/core/MyTransportRFM69.cpp#L66

And report if RX/TX indications work?

@janchrillesen

This comment has been minimized.

Copy link
Contributor Author

janchrillesen commented Sep 20, 2016

RX/TX indication works fine, as the LED's are blinking as expected on the gateway. I will check the code change tonight

@janchrillesen

This comment has been minimized.

Copy link
Contributor Author

janchrillesen commented Sep 21, 2016

Hi tekka007

The change above does not fix the issue. I looked at bit at MyIndication.cpp last night and clearly this code is reached since the TX/RX LED's are flashing as expected. As I understand this is also where the indication() call to the gateway sketch is done.

@janchrillesen

This comment has been minimized.

Copy link
Contributor Author

janchrillesen commented Sep 21, 2016

I did some testing - I added a few lines of debug code to MyIndication.cpp and now it works:

Basically I changed

    if (indication)
        indication(ind);

to

    debug(PSTR("DEBUG::indication-main\n"));
    if (indication) {
        debug(PSTR("DEBUG::indication-debug\n"));
        indication(ind);
    }

That is the only change - so no functional change, but still it works now. I see callback of type 1, 2 and 3 to the indication() function in my main sketch and both TX and RX counters works in the webinterface.
I have no idea what's going on here but it seems to be timing related, as the following also works

    sleep(10);
    if (indication) {
    indication(ind);

As I have no idea why this works I'll leave it to someone else to do a pull request

@Yveaux

This comment has been minimized.

Copy link
Member

Yveaux commented Sep 23, 2016

Sleep() is not implemented on Esp8266. It does however print to serial using the debug-macro and flushes the serial device.
Could you try printing something using debug() from your indication() implementation (with original library) and see if that does print?

@tekka007

This comment has been minimized.

Copy link
Contributor

tekka007 commented Sep 24, 2016

@Yveaux I assume this is again related to the code optimization bug present in gcc < 6.1. It has been fixed in 6.3 and back-ported to 5.5, but will take some time until avr toolchan is updated.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77326

@janchrillesen

This comment has been minimized.

Copy link
Contributor Author

janchrillesen commented Sep 25, 2016

@Yveaux Adding debug() doesn't change anything - the indication() function is now like this:

void indication( const indication_t ind )
{
  debug("INDICATION");
  switch (ind)
  {
    case INDICATION_GW_TX:
      gwMsgSent++;
      break;

    case INDICATION_GW_RX:
      gwMsgRec++;
      break;

    default:
    break;
  };
}

Changing sleep(10) to delay(10) in MyIndication.cpp works fine.

@tekka007 If this bug is AVR related it shouldn't matter on an ESP8266, as this is not an AVR platform?
Also, if this is a compiler bug I don't see why it works with NRF24 and not with RFM69, as the indication code doesn't seem to have any dependencies to the transport method

@tekka007

This comment has been minimized.

Copy link
Contributor

tekka007 commented Sep 25, 2016

@janchrillesen The optimization bug I'm referring to is gcc related - and both, AVR and esp8266, use gcc ~4.8.x (at least on Arduino 1.6.12).
Out of curiosity, what happens if you use the SerialGateway with RFM69 on AVR, does the indication work as expected?

@janchrillesen

This comment has been minimized.

Copy link
Contributor Author

janchrillesen commented Sep 30, 2016

@tekka007 I will need to build a seperate AVR based serial gateway to test this. For now I'll just manually edit MyIndication.cpp until a more recent gcc is available in the ESP8266 package. As it's pretty clear that this is a gcc issue, and not MySensors related I'll close the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.