feat: support arbitrary APPNAME and correct GPIOs #69
Conversation
Closes #56 - renamed test file for consistency with underlying class - added hardware definitions dependency and load it in app - updated naming logic based on variant details
|
Working well on a RAK based hotspot and light Hotspot Check small-smoke on helium-light-test |
- get rid of isIndoorVariant helper - add Mock.GPIO test dependency - fix an unrelated issue with setting miner_keys when they are malformed, discovered during testing. - having difficulty testing gatewayconfig_app because state is not being properly cleared between tests
marvinmarnold
changed the title
| status_led_gpio = OTHER_STATUS_LED_GPIO | ||
|
|
||
| self.user_button = Button(user_button_gpio, hold_time=USER_BUTTON_HOLD_SECONDS) | ||
| self.user_button = Button(self.get_button_pin(), hold_time=USER_BUTTON_HOLD_SECONDS) |
There was a problem hiding this comment.
get_button_pin created to make unit testing easier
| @@ -60,7 +62,7 @@ def stop(self): | |||
| logger.debug("Stopping ConfigApp") | |||
| GPIO.cleanup() | |||
| # Quits the cputemp application | |||
| self.bluetooth_processor.quit() | |||
| self.bluetooth_services_processor.quit() | |||
There was a problem hiding this comment.
This is an unrelated bug discovered during unit testing
| @@ -35,7 +35,10 @@ def read_miner_keys(miner_keys_filepath): | |||
| for key in json_line.keys(): | |||
| miner_keys[key] = json_line[key] | |||
| except json.JSONDecodeError: | |||
| miner_keys[key] = False | |||
| logger.error("Unable to correctly decode miner_keys. The app will continue running but may not behave as expected.") | |||
| miner_keys['pubkey'] = False | |||
There was a problem hiding this comment.
We should talk about this behavior more. The current logic will allow the application to start-up if the miner_keys file exists, even if it is malformed. Later on the app will choke when it tries to use those keys. Should we instead exit immediately when malformed JSON? Exponential backoff?
There was a problem hiding this comment.
I'm not sure. In one sense it would be better for it not to work.
However from a customers perspective it would be better for it to show a (useful) error on the app IMO
There was a problem hiding this comment.
Created #80 to follow-up with this.
|
@vpetersson can we merge this? |
|
@vpetersson @marvinmarnold pushed to testnet for testing NebraLtd/helium-miner-software@2ef36bd |
|
Seems to be working fine for me |
Why
When advertising, we pull the APPNAME parameter from the variant definitions file. However, some of the variants (non-Nebra and DIY) do not have an APPNAME which causes this to fail.
Also, different variants use different GPIOs, which need to be correctly used.
How
malformed, discovered during testing.
not being properly cleared between tests
References
Closes #56
Closes #58