Skip to content
This repository has been archived by the owner on Jun 12, 2023. It is now read-only.

Non-chain patch to PoC spreading factor choice. #1865

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ke6jjj
Copy link
Contributor

@ke6jjj ke6jjj commented Dec 8, 2022

This PR is mutually exclusive with helium/blockchain-core#1499

@mikev says:

Recently we've taken a close look at PoC in the context of AS923. Network observations show that AS923 PoC is beaconing at SF9. As a comparison EU868 successfully beacons at SF12. We looked deeper into the Miner's PoC code and believe this is incorrect. The spec allows 400ms dwelltime regions, such as AS923, to use the SF12 datarate for packets 59 bytes and smaller. Our current PoC packets are 51 52 bytes.

We propose a minor patch to enable the AS923 regions to PoC beacon at the SF12 datarate. This will have the effect of approximately doubling the PoC distance range. The PoC distance range roughly matches the distance a LoRa sensor can achieve when it sends it's initial Join request at SF12.

@ke6jjj ke6jjj changed the title Stage a possible patch to PoC spreading factor choice. Non-chain patch to PoC spreading factor choice. Dec 8, 2022
@Vagabond
Copy link
Contributor

Vagabond commented Dec 8, 2022

shouldn't this be done via the region parameters chain var? @vihu ?

@mikev mikev requested a review from vihu December 8, 2022 21:08
@vihu
Copy link
Member

vihu commented Dec 8, 2022

shouldn't this be done via the region parameters chain var? @vihu ?

Yeah, that would be ideal. @ke6jjj @mikev can you take a look at https://github.com/helium/blockchain-core/blob/master/test/blockchain_region_test.hrl file and update it according to the new spec, I assume only the spreading values need to change there? We can then use the changes and update the region_as923_1_params, region_as923_2_params etc accordingly via chain var.

@ke6jjj
Copy link
Contributor Author

ke6jjj commented Dec 8, 2022

We are structuring the change this way precisely to avoid doing a chain var. The fact of the matter is that chain var changes are very risky at the moment and I don't feel we're comfortable making them so close to a holiday period.

@vihu
Copy link
Member

vihu commented Dec 8, 2022

We are structuring the change this way precisely to avoid doing a chain var. The fact of the matter is that chain var changes are very risky at the moment and I don't feel we're comfortable making them so close to a holiday period.

Fair point. However, this would be a chain var for region_X_params and not a region change. I believe those are not soo risky since there's no cache cleanup required. Thoughts @Vagabond ?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants