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

Add light hotspot automation #411

Closed
wants to merge 2 commits into from

Conversation

MuratUrsavas
Copy link
Contributor

Issue
#399

  • Summary:
    Adapting all automation to light hotspots.

How

  • Removed dbus, hm-miner container references
  • Added hm-gatewayrs references

Checklist

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

@MuratUrsavas MuratUrsavas requested review from a team as code owners March 28, 2022 18:17
@MuratUrsavas MuratUrsavas marked this pull request as draft March 28, 2022 18:19
@MuratUrsavas MuratUrsavas changed the base branch from master to light-hotspot March 28, 2022 18:19
@MuratUrsavas MuratUrsavas marked this pull request as ready for review March 28, 2022 18:20
@MuratUrsavas MuratUrsavas changed the title Murat/light hotspot automation Add light hotspot automation Mar 28, 2022
@@ -74,13 +46,13 @@ jobs:
if: matrix.sbc == 'rockpi' && success()
with:
balena_api_token: ${{secrets.BALENA_API_TOKEN}}
balena_command: "deploy nebraltd/helium-${{ matrix.variant }}-${{ matrix.frequency }}${{ env.ROCKPI }} --logs --debug --nocache --build --draft"
balena_command: "deploy nebraltd/helium${{ env.ROCKPI }} --logs --debug --nocache --build --draft"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remind me where env.ROCKPI is being set and what values it takes on?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- name: Balena Deploy (Nebra RasPi)
uses: nebraltd/[email protected]
if: matrix.sbc == 'raspi' && success()
with:
balena_api_token: ${{secrets.BALENA_API_TOKEN_1}}
balena_command: "deploy nebraltd/helium-${{ matrix.variant }}-${{ matrix.frequency }} --logs --debug --nocache --build --draft"
balena_command: "deploy nebraltd/helium-light-hotspot --logs --debug --nocache --build --draft"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be helium-light-test? I'm currently using helium-light-hotspot to test this flow which is different.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was for production, not testnet.

@@ -10,34 +10,6 @@ jobs:
fail-fast: false
matrix:
sbc: [raspi, rockpi, rak, pisces, og, sensecap, finestra, controllino]
frequency: [470, 868, 915]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want to touch the prod workflows? Won't those be unchanged?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're removing indoor outdoor variants and all frequencies. So, not needed to keep all of matrix rows.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we had decided that fleet reorganization will be separate ticket. if that matter has been settled, then its good.

@@ -41,19 +41,19 @@ jobs:
if: matrix.sbc == 'rockpi' && success()
with:
balena_api_token: ${{secrets.BALENA_API_TOKEN}}
balena_command: "deploy nebraltd/helium-testnet${{ env.ROCKPI }} --logs --debug --nocache --build --draft"
balena_command: "deploy nebraltd/helium-light-testnet${{ env.ROCKPI }} --logs --debug --nocache --build --draft"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about consolidating all of these into a single alena_command: "deploy nebraltd/helium-light-testnet-${{ matrix.sbc }} --logs --debug --nocache --build --draft"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need more than one testnet for light? Probably overkill just for testing

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also it should be helium-light-test fleet that already exists

Take a look at the light-hotspot-software repo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about consolidating all of these into a single alena_command: "deploy nebraltd/helium-light-testnet-${{ matrix.sbc }} --logs --debug --nocache --build --draft"?

That could be possible. Let me think about our reasons to keep it separate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was my idea. We'll need to have separate fleets for every device type. I thought it has to mimic the same environment for testing. Also this is a one time setup. Otherwise, we'll be chasing different things for each device on testnet. That could need more effort than this one. At least this was my humble opinion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also it should be helium-light-test fleet that already exists

Take a look at the light-hotspot-software repo

OK. I can use helium-light-test. But AFAIK we had abondoned light-sotspot-software and using light-hotspot branch of helium-miner-software?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we are, but essentially we are migrating from that older repo to this one to make it easier for us to merge this to master branch when it goes live.

So it needs to be done in a way which can easily be merged with master further down the line.

Also, we aren't going to move devices to new fleets for light. We will just update the existing fleets with the newer software, when the time comes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK.

@shawaj
Copy link
Member

shawaj commented Mar 29, 2022

IIRC GitHub actions workflows will only work from the master branch

@shawaj
Copy link
Member

shawaj commented Mar 29, 2022

Some of this is duplicated in #409

Also you can't rename the helium-miner container in the yml file or it will break packet forwarder and hm-pyhelper I think

restart: always
privileged: true
volumes:
- pktfwdr:/var/pktfwd
environment:
- FIRMWARE_VERSION={{FIRMWARE_VERSION}}

helium-miner:
image: nebraltd/hm-gatewayrs:{{GATEWAY_VERSION}}
gateway-rs:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment about why this can't be changed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It shouldn't be a problem, but I'm not sure. I've removed the dependency, but will make sure packet-forwarder doesn't fail.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we will have to keep helium-miner. it will need changes at too many places. Not important right now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it would need changing in packet forwarder and in hm-pyhelper and not worth the effort.

Also having it called helium-miner is still technically correct

environment:
- FIRMWARE_VERSION={{FIRMWARE_VERSION}}
- FIRMWARE_SHORT_HASH={{ENV.FIRMWARE_SHORT_HASH}}
- DIAGNOSTICS_VERSION={{DIAGNOSTICS_VERSION}}
- DBUS_SYSTEM_BUS_ADDRESS=unix:path=/host/run/dbus/system_bus_socket
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still required

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure about that. @pritamghanghas What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

system bus is still needed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need it for Bluetooth / LTE / WiFi

environment:
- FIRMWARE_VERSION={{FIRMWARE_VERSION}}
- FIRMWARE_SHORT_HASH={{ENV.FIRMWARE_SHORT_HASH}}
- DBUS_SYSTEM_BUS_ADDRESS=unix:path=/host/run/dbus/system_bus_socket
- DBUS_SESSION_BUS_ADDRESS=unix:path=/session/dbus/session_bus_socket
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

System bus is still required I believe

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above. This was a thing I'd like to discuss while the PR is open.

labels:
io.balena.features.sysfs: 1
io.balena.features.kernel-modules: 1
io.balena.features.dbus: 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

System bus still needed I think

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -79,19 +71,7 @@ services:
labels:
io.balena.features.sysfs: 1
io.balena.features.procfs: 1
io.balena.features.dbus: 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still needed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@vpetersson
Copy link
Contributor

IIRC GitHub actions workflows will only work from the master branch

No this isn't true. You can run them on any branch. We already do this in multiple repos.

@shawaj
Copy link
Member

shawaj commented Mar 29, 2022

IIRC GitHub actions workflows will only work from the master branch

No this isn't true. You can run them on any branch. We already do this in multiple repos.

@vpetersson you can't run scheduled actions from anything but master branch

They can run on another branch. But the workflow itself has to be stored in master branch for it to appear in the "actions" tab and for you to be able to do scheduled, repository dispatch or workflow dispatch based actions.

@MuratUrsavas MuratUrsavas deleted the murat/light-hotspot-automation branch March 30, 2022 06:30
pritamghanghas added a commit that referenced this pull request Jul 15, 2022
* update hm-diag hash containing this message.
pritamghanghas added a commit that referenced this pull request Jul 15, 2022
feat: Messaging for disabled blockchain sync #411
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 this pull request may close these issues.

5 participants