fix: remove unnecessary ECC lock and improve bus check #234
Conversation
shawaj
changed the title
shawaj
changed the title
- 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 Ref: NebraLtd/helium-syncrobit#10 Ref: NebraLtd/helium-syncrobit#2
| with open("eccfile", 'r') as data: | ||
| generated_ecc_location = str(data.read()).rstrip('\n') | ||
| if len(generated_ecc_location) < 10: | ||
| generated_ecc_location = None |
There was a problem hiding this comment.
it works but I always find it confusing reading python code that defines a variable in if control block and then uses it outside.
There was a problem hiding this comment.
what would you suggest instead?
There was a problem hiding this comment.
declaration of variable before the control block is something that makes it much more readable.
There was a problem hiding this comment.
generated_ecc_location = str(data.read()).rstrip('\n') is defined in the line above initially? Or am I misunderstanding what you mean?
There was a problem hiding this comment.
@pritamghanghas any clarification here?
There was a problem hiding this comment.
nothing really. I was just pointing to the weird scoping in python. I had approved it. This is fine.
|
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. |
|
Kudos, SonarCloud Quality Gate passed!
|
Issue
How
Screenshots
References
Ref: NebraLtd/helium-syncrobit#10
Ref: NebraLtd/helium-syncrobit#2
Checklist