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

InteractiveRouter not sending synchronous ack for packets directed at it #292

Closed
chestwood96 opened this issue Apr 28, 2019 · 7 comments
Closed
Labels

Comments

@chestwood96
Copy link
Contributor

It appears the InteractiveRouter does not send a synchronous ack when it receives the packet itself.

I have worked around that by sending it myself right now but should it not do it itself?

@gioblu
Copy link
Owner

gioblu commented Apr 29, 2019

Ciao @chestwood96 thank you very much.
I have discussed this with @fredilarsen on gitter, and we agree, the InteractiveRouter being a class that wants to implement the features of the router along with the ones of the device, it should send the synchronous acknowledge automatically if the packet is for its own device id, as you suggest.

I will apply the change as soon as possible..
Thank you again.

@chestwood96
Copy link
Contributor Author

While you are at it it would be nice to have a regular PJON-Like send function for the interactive router.
The current one kind of requires you to either build the whole packet or know which bus to send to.

Should i make a separate issue for this?

@gioblu
Copy link
Owner

gioblu commented Apr 30, 2019

Don't be preoccupied @chestwood96 I will add the overload to the functions you need, thank you for pointing out.

@gioblu
Copy link
Owner

gioblu commented May 3, 2019

Ciao @chestwood96 I have added the ack fix you have proposed.

@fredilarsen how would you implement the PJON-like send functions?

I think @chestwood96 is right to expect to find them, being the InteractiveRouter also a device, should provide the functions to send normally as a device, although those functions should leverage of the router features.

I have made some experiments, I suspect they should:

  • If the bus and device id is found in the list of buses, send directly
  • If not, if there is a default gateway, send to default gateway
  • if not, should do nothing and return an error? Or flood?

@fredilarsen
Copy link
Contributor

fredilarsen commented May 10, 2019

@gioblu, @chestwood96 There already is a PJONInteractiveRouter::send_packet which should do this:

void send_packet(
    const uint8_t *payload,
    uint16_t length,
    const PJON_Packet_Info &packet_info
  )

It requires you to declare a local Packet_Info object, but that only contains the parameters needed for sending anyhow (from_id, to_id, potential bus ids etc).

There is another send_packet in there as well which requires the target bus to be specified, I guess that was the one you referred to?

@chestwood96
Copy link
Contributor Author

I was actually using router.get_bus(router.get_callback_bus()) for the testing.

I kind of looked at PJON_Packet_Info like something pjon internal I should not be messing with from the outside.

@gioblu
Copy link
Owner

gioblu commented Aug 11, 2019

Ciao @chestwood96 at the end we understood that was necessary, as you correctly pointed out. Here is the commit that fixes it 9fc81b3

Soon v12.0 will be released including this optimization.
Thank you for raising this.

@gioblu gioblu closed this as completed Aug 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants