Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Need to support hardware variants with multiple possible ECC locations #221

Open
shawaj opened this issue Feb 12, 2023 · 6 comments
Open

Comments

@shawaj
Copy link
Member

shawaj commented Feb 12, 2023

It turns out that Syncrobit miners have several possible locations for the ECC key on the i2c bus...

NebraLtd/helium-syncrobit#2

We should add handling for this scenario

@pritamghanghas
Copy link
Contributor

mfr throws exec error if we try to read a slot that is not programmed before.

root@78eef93a415c:/opt# ./python-dependencies/hm_pyhelper/gateway_mfr_aarch64 --device 'ecc://i2c-7:96?slot=5' key
Error: ecc608 error

Caused by:
    ecc error ExecError

Between i2cdetect and cycling through the slots, we can probably find out the correct combo. Assuming that multiple slots were not programmed.

@shawaj
Copy link
Member Author

shawaj commented Feb 17, 2023

@pritamghanghas sadly I've found that sometimes multiple are programmed. See output here...

NebraLtd/helium-syncrobit#2 (comment)

shawaj added a commit that referenced this issue Mar 11, 2023
This is a quick and hacky fix to the issue described here NebraLtd/helium-syncrobit#2 (comment)

The idea is to allow a device level environment variable in balenaCloud to override the key location in the assigned variant without needing to define a new variant. This is useful for testing new devices or in instances like this Syncrobit one to provide a quick fix until we have implemented a "proper" fix to this issue.

This additionally changes the following:
- we hard code the gateway_mfr_rs version in the [test](https://github.com/NebraLtd/hm-pyhelper/blob/master/.github/workflows/publish-to-pypi-test.yml) and [production](https://github.com/NebraLtd/hm-pyhelper/blob/master/.github/workflows/publish-to-pypi.yml) python package build actions, so there is no need to support old versions of gateway-mfr-rs
- due to removing the above, there is also no need to keep the KEY_STORAGE_BUS key in the hardware variants as this is not used anywhere else that I'm aware of **(we should double check this in config, diag, miner, packet forwarder and multiplexer containers just to be 100% sure)**
  - [Used as a fallback in diag](https://github.com/NebraLtd/hm-diag/blob/23c66980f75568a4cecb18c5375a08cd67bff550/hw_diag/utilities/hardware.py#L200) but not required

Relates-to: NebraLtd/helium-syncrobit#2
Relates-to: #221
Relates-to: #222
@shawaj
Copy link
Member Author

shawaj commented Mar 11, 2023

Also, currently the i2c address on the bus is hard coded in diagnostics - https://github.com/NebraLtd/hm-diag/blob/master/hw_diag/utilities/hardware.py#L235

This address is 0x60 / decimal 96 which is in the swarm key uri already for example ecc://i2c-1:96?slot=0

We should change this also at same time

Ref - NebraLtd/hm-diag#500

Also see https://nebraltd.slack.com/archives/C024BNQ1Y6T/p1678578139710139

shawaj added a commit that referenced this issue Mar 11, 2023
This is a quick and hacky fix to the issue described here NebraLtd/helium-syncrobit#2 (comment)

The idea is to allow a device level environment variable `SWARM_KEY_URI_OVERRIDE` in balenaCloud to override the key location in the assigned variant without needing to define a new variant. This is useful for testing new devices or in instances like this Syncrobit one to provide a quick fix until we have implemented a "proper" fix to this issue.

This additionally changes the following:
- we hard code the gateway_mfr_rs version in the [test](https://github.com/NebraLtd/hm-pyhelper/blob/master/.github/workflows/publish-to-pypi-test.yml) and [production](https://github.com/NebraLtd/hm-pyhelper/blob/master/.github/workflows/publish-to-pypi.yml) python package build actions, so there is no need to support old versions of gateway-mfr-rs
- due to removing the above, there is also no need to keep the KEY_STORAGE_BUS key in the hardware variants as this is not used anywhere else that I'm aware of **(we should double check this in config, diag, miner, packet forwarder and multiplexer containers just to be 100% sure)**
  - [Used as a fallback in diag](https://github.com/NebraLtd/hm-diag/blob/23c66980f75568a4cecb18c5375a08cd67bff550/hw_diag/utilities/hardware.py#L200) but not required

Relates-to: NebraLtd/helium-syncrobit#2
Relates-to: #221
Relates-to: #222
@shawaj
Copy link
Member Author

shawaj commented Mar 12, 2023

This will also need significant updates in:

  • hm-miner
  • hm-block-tracker

shawaj added a commit that referenced this issue Mar 13, 2023
This is a quick and hacky fix to the issue described here NebraLtd/helium-syncrobit#2 (comment)

The idea is to allow a device level environment variable `SWARM_KEY_URI_OVERRIDE` in balenaCloud to override the key location in the assigned variant without needing to define a new variant. This is useful for testing new devices or in instances like this Syncrobit one to provide a quick fix until we have implemented a "proper" fix to this issue.

This additionally changes the following:
- we hard code the gateway_mfr_rs version in the [test](https://github.com/NebraLtd/hm-pyhelper/blob/master/.github/workflows/publish-to-pypi-test.yml) and [production](https://github.com/NebraLtd/hm-pyhelper/blob/master/.github/workflows/publish-to-pypi.yml) python package build actions, so there is no need to support old versions of gateway-mfr-rs
- due to removing the above, there is also no need to keep the KEY_STORAGE_BUS key in the hardware variants as this is not used anywhere else that I'm aware of **(we should double check this in config, diag, miner, packet forwarder and multiplexer containers just to be 100% sure)**
  - [Used as a fallback in diag](https://github.com/NebraLtd/hm-diag/blob/23c66980f75568a4cecb18c5375a08cd67bff550/hw_diag/utilities/hardware.py#L200) but not required

Relates-to: NebraLtd/helium-syncrobit#2
Relates-to: #221
Relates-to: #222
shawaj added a commit that referenced this issue Mar 13, 2023
…227)

* feat: allow override of key location and remove old version support

This is a quick and hacky fix to the issue described here NebraLtd/helium-syncrobit#2 (comment)

The idea is to allow a device level environment variable `SWARM_KEY_URI_OVERRIDE` in balenaCloud to override the key location in the assigned variant without needing to define a new variant. This is useful for testing new devices or in instances like this Syncrobit one to provide a quick fix until we have implemented a "proper" fix to this issue.

This additionally changes the following:
- we hard code the gateway_mfr_rs version in the [test](https://github.com/NebraLtd/hm-pyhelper/blob/master/.github/workflows/publish-to-pypi-test.yml) and [production](https://github.com/NebraLtd/hm-pyhelper/blob/master/.github/workflows/publish-to-pypi.yml) python package build actions, so there is no need to support old versions of gateway-mfr-rs
- due to removing the above, there is also no need to keep the KEY_STORAGE_BUS key in the hardware variants as this is not used anywhere else that I'm aware of **(we should double check this in config, diag, miner, packet forwarder and multiplexer containers just to be 100% sure)**
  - [Used as a fallback in diag](https://github.com/NebraLtd/hm-diag/blob/23c66980f75568a4cecb18c5375a08cd67bff550/hw_diag/utilities/hardware.py#L200) but not required

Relates-to: NebraLtd/helium-syncrobit#2
Relates-to: #221
Relates-to: #222
shawaj added a commit that referenced this issue Mar 15, 2023
Syncrobit has multiple potential key locations, change to list and parse list if necessary

Relates-to: #221
Relates-to: NebraLtd/helium-syncrobit#2 (comment)
Supersedes: #222
shawaj added a commit that referenced this issue Mar 15, 2023
Syncrobit has multiple potential key locations, change to list and parse list if necessary

Relates-to: #221
Relates-to: NebraLtd/helium-syncrobit#2 (comment)
Supersedes: #222
@shawaj
Copy link
Member Author

shawaj commented Mar 15, 2023

This PR is a partial fix for Syncrobit multi key issue:
#230

This will solve the vast majority of cases. However some will still need to be overridden with a SWARM_KEY_URI_OVERRIDE device variable currently.

In order to provide a complete fix, we still need to implement the following:

  • if we have definitions in hardware variants for multiple different slots on the same i2c address then:
    • Check keys on public API. If one is found, then use that one
    • If none of the available keys are found on public API, check the onboarding server
    • If only one is found, use that one
    • If multiple found on onboarding server, use the first valid one for onboarding

We also need to pythonise the miner start scripts (or portions of it) to override the keyslot in the sys.config. Or to publish the determined key as an environment variable that the miner can ingest... But that seems more convoluted.

Ref https://github.com/syncrobit/chameleonos/blob/main/board/common/overlay/etc/init.d/S31hwdetect

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

Successfully merging a pull request may close this issue.

2 participants