[WIP] Implement sending welcome packets to clickers #16
Conversation
ammaraskar
changed the title
| uint8_t checksum = 0; | ||
| for (int i = 0; i < WELCOME_PACKET_SIZE - 1; i++) { | ||
| checksum += welcome_payload[i]; | ||
| } |
There was a problem hiding this comment.
Can't this just be done with the computeChecksum method?
|
|
||
| #define WELCOME_RECV_SYNC_ADDR_LEN 6 | ||
| const uint8_t WELCOME_RECV_SYNC_ADDR[WELCOME_RECV_SYNC_ADDR_LEN] = | ||
| {0x55, 0x55, 0x55, 0x36, 0x36, 0x36}; |
There was a problem hiding this comment.
I imagine you see the base station's firmware transmitting this leading 0x55 . Remember 0x55 = 0b01010101.
This is used to help the receiver learn the channel, not to actually try to trigger on using the sync address functionality. You can not reliable assume it will be received {0x55, 0x55, 0x55}. The sync address should just be {0x36, 0x36, 0x36}. Also remember when calling _radio.send(msg) the RFM69 is automatically creating the packet to transmit as {0x55, 0x55, ..., 0x55} + sync_addr + msg, as it automatically prepends the 0x55 preamble and the sync address currently configured.
This actually bring up a problem with the way thehttps://github.com//pull/12. Trying to receive the ack by triggering using the sync address functionality on the preamble ({0x55, 0x55, 0x55}) likely will not work reliably in certain situations. I think we should use the first two bytes of the encodedID instead.
There was a problem hiding this comment.
as it automatically prepends the 0x55 preamble
The RFM69HCW's datasheet is really confusing me here, the only configurable part of the preamble seems to be RegPreambleMsb and RegPreambleLsb which defaults to 0x03, so is the preamble hardcoded to just be 0x55? Is this just an industry convention or something?
Trying to receive the ack by triggering using the sync address functionality on the preamble ({0x55, 0x55, 0x55}) likely will not work reliably in certain situations. I think we should use the first two bytes of the encodedID instead.
The datasheet does mention the following:
- A preamble (0x55 or 0xAA) of 12 bits is required for synchronization (from the RxReady interrupt)
So only 1.5 bytes are required to trigger the preamble detecting part of the RFM69 and the rest should be safe to use as a sync word? Also won't this present a problem for promiscuously snooping on all ACKs? What do we use as the sync word there?
There was a problem hiding this comment.
the preamble hardcoded to just be 0x55? Is this just an industry convention or something?
Yeah, the datasheet is a bit confusing... so the preamble is an alternating series of 1 and 0s. So both 0b01010101 (0x55) as well as 0b10101010 (0xaa) would work.
So only 1.5 bytes are required to trigger the preamble detecting part of the RFM69 and the rest should be safe to use as a sync word?
1.5 bytes is only the minimum. I'm not sure if with this particular chipset a longer preamble helps after the first 1.5 bytes. After reading the datasheet more carefully it looks like it is mostly being used to lock on to the bitrate of the transmission. Remember this is supposed to be a general purpose FSK transceivers, so they designed it to be compatible with other transceivers. Some more advanced FSK receivers could use this preamble to do additional things.
I think the best solution is just avoid using preamble bytes for the sync word unless we absolutely have to. For example with answer submission the sync word is {0x85, 0x85, 0x85} not {0x55, 0x55, 0x55, 0x85, 0x85, 0x85,}. We do not need to use sync bytes in this case.
Also won't this present a problem for promiscuously snooping on all ACKs? What do we use as the sync word there?
Yeah, this is a problem with trying to support promisoucMode for acks. See #13
| @@ -111,6 +149,8 @@ class iClickerEmulator | |||
| bool begin(iClickerChannel chan); | |||
| bool submitAnswer(uint8_t id[ICLICKER_ID_LEN], iClickerAnswer ans, | |||
| bool withAck=false, uint32_t timeout=DEFAULT_ACK_TIMEOUT, bool waitClear = true); | |||
| void sendWelcomePacket(char* msg, QuestionMode mode=QuestionModes::MULTIPLE_CHOICE, | |||
There was a problem hiding this comment.
const char* so a vanilla C string works.
@wizard97 Lemme know if I used the
_RECV/_SENDconvention right here, I had to expand it out since other packets will also have varying lengths.@charlescao460 Would appreciate you testing this with a base station, made a change to the way the radio module is used so we don't have to hardcode payload lengths. Also, you can try out the undocumented(?) question modes.