fix: remove unnecessary ECC lock and improve bus check

NebraLtd / hm-pyhelper

A helper module used across various Nebra repos to reduce redundant features.

https://nebra.io/hmpyhelper

MIT License

9 stars 16 forks source link

fix: remove unnecessary ECC lock and improve bus check #234

Closed shawaj closed 1 year ago

shawaj commented 1 year ago

Issue

Link: https://github.com/NebraLtd/helium-syncrobit/issues/10
Summary: remove unnecessary ECC lock and improve bus check

How

ECC lock not required on this check because all we are doing is checking the i2c address of the attached device.
persist a generated ECC location (from the i2c bus) to file to make get_ecc_location() function more efficient and to remove unnecessary calls to the i2c bus
additional tests for SWARM_KEY_URI_OVERRIDE and new "persist to file" feature

Screenshots

References

Ref: https://github.com/NebraLtd/helium-syncrobit/issues/10 Ref: https://github.com/NebraLtd/helium-syncrobit/issues/2

Checklist

[x] Tests added
[x] Cleaned up commit history (rebase!)
[x] Documentation added
[x] Thought about variable and method names

pritamghanghas commented 1 year ago

I don't think ecc_lock is required anywhere. I think the thought process behind that lock was flawed. It is used like it is an os semaphore while it is only a lock within the application.

No need to remove it though, I don't think it is hurting anything either.

shawaj commented 1 year ago

I don't think ecc_lock is required anywhere. I think the thought process behind that lock was flawed. It is used like it is an os semaphore while it is only a lock within the application.

No need to remove it though, I don't think it is hurting anything either.

I think the reason we put it in was because it was clashing with itself when trying to run from different places and these collisions were causing unexpected failures. However...we haven't actually really tested gateway-mfr-rs stuff without the ecc lock, only original erlang based gateway_mfr IIRC. So probably it is better now.

Definitely only think it makes sense when we are actually performing operations on the ecc though, and not when we are just checking its address on the i2c bus. But as you say, if not causing any issues, probably easier to leave it in.

sonarcloud[bot] commented 1 year ago

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication