search

anthonykirby / lora-packet

LoRa radio packet decoder
MIT License
258 stars 83 forks source link

Refactoring to improve readabilty and modularity #113

Closed taraskuzyk closed 9 months ago

taraskuzyk commented 10 months ago

I saw a lot of repeated snippets of code so wanted to give a crack at refactoring out into something more "clean", which is subjective so I'm open to feedback. The main changes are related to standardizing the buffer bytes extraction. I'd like to continue work on this as long as I know that the direction I'm taking the code in aligns with what your expectations for the project are. Tests in the test suite have all passed in each of the commits in this PR.

Through this process, I noticed that for REJOIN_TYPE_2 RJCount1 is overlapping with DevEUI which can't be right:

REJOIN_TYPE_2: {
    JoinEUI: {start: 2, end: 10},
    DevEUI: {start: 10, end: 18},
    RJCount1: {start: 13, end: 15}
}

Bugs like these would be far easier to track down and avoid making in the first place if we extracted all the repeated lines of code into functions and structures, e.g. PACKET_STRUCTURES constant.

anthonykirby commented 10 months ago

hi @taraskuzyk that looks great, thank you. Could you fix the linter errors please (github checks run npm run linter) & I'd be happy to merge.

(I don't know why the github checks dont't run automatically on PRs; I managed to make them run by creating a branch with the same commit hash :shrug:)

taraskuzyk commented 10 months ago

Merged your new changes into the branch and ran the linter. Let's see if it goes through this time.

anthonykirby commented 9 months ago

the linter still isn't happy, but the changes are good and the tests are otherwise passing so I'll merge this & then placate the linter in a subsequent PR