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

Phantom device (change BROADCAST ID) #40

Closed
thomsna opened this issue Mar 31, 2016 · 6 comments
Closed

Phantom device (change BROADCAST ID) #40

thomsna opened this issue Mar 31, 2016 · 6 comments
Labels

Comments

@thomsna
Copy link

thomsna commented Mar 31, 2016

Two Arduino pro mini ATmega328.
A master set at address 45 and a slave set at 44.
Bus on pin 12.

While sending to device 124 even though it didn't exist on the network, I get ACK values back.

Can someone reproduce this?

Here, I wrote a script to check for a response on all addresses.
Code for master:

#include <PJON.h>

PJON network(12, 45);

char content[] = "ajtyenflhiehfnvk";

void setup() {
    Serial.begin(115200);
    delay(250);
    uint8_t test = 0;
    long scan_time;

    for (int i=0; i<256; i++) {
        scan_time = millis();
        while (millis() - scan_time < 20) {
        int response = network.send_string(i, content, 16);
            if (response == ACK) {
                test++;
            }
        }
    if (test > 0) Serial.println(i);        
    test = 0;
    }
}

void loop() {};

Code for slave:

#include <PJON.h>

PJON network(12, 44);

void setup() {
  Serial.begin(115200);
};

void loop() {
  network.receive(); 
};
@aperepel
Copy link
Contributor

@thomsna if I read the code correctly, address 124 is the broadcast. In your teat, try counting how many acks you get and from what addresses to confirm.

@thomsna
Copy link
Author

thomsna commented Mar 31, 2016

@aperepel Thanks for clearing that up. 124 is indeed the broadcast address.
The code above will check for the ACK from a device on all addresses. An ACK is received on address 124, even though no device is set to that address.

@aperepel
Copy link
Contributor

Great, this is not an issue, it looks. I would maybe prefer to have the broadcast on a more 'classic' address like 0 or 255, than in a middle of the range. @gioblu any comment on that? 124 doesn't even look cool in binary :)

@thomsna
Copy link
Author

thomsna commented Mar 31, 2016

+1 for setting broadcast address to 0, like I2C.
It would also make the code clearer in my situation to count from 1 to 255, instead of taking care of excluding address 124.

@gioblu
Copy link
Owner

gioblu commented Mar 31, 2016

Hi @aperepel @thomsna to be consistent and not subjected to versioning changes (as the one you are proposing) bugs, always use constants if you can: for(uint8_t i = 0; i < 256; i++) if(i == BROADCAST) continue; so if BROADCAST value will change during development this will not affect your sketches.

Could be more clean I agree, we should run extended tests, but if all is ok, I don't see limits in changing BROADCAST to 0. Thank you again for your support and passion in this project 👍

@gioblu gioblu added the TODO label Mar 31, 2016
@gioblu gioblu changed the title Phantom device Phantom device (change BROADCAST id) Mar 31, 2016
@gioblu gioblu changed the title Phantom device (change BROADCAST id) Phantom device (change BROADCAST ID) Mar 31, 2016
@gioblu
Copy link
Owner

gioblu commented Apr 7, 2016

Ciao @aperepel, @thomsna.
With the release of the 2.0 version this issue has been fixed and BROADCAST id is now 0, as correctly proposed 👍 . Thank you again for your support and passion in this project.

@gioblu gioblu closed this as completed Apr 7, 2016
@gioblu gioblu added bug and removed TODO labels Apr 25, 2016
gioblu pushed a commit that referenced this issue Mar 8, 2019
Merged from gioblu master
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