Ribbit-Network / ribbit-network-frog-hardware

The sensor for the world's largest crowdsourced network of open-source, low-cost, GHG Gas Detection Sensors.
https://www.ribbitnetwork.org/
MIT License
94 stars 26 forks source link

Add support for adding config to BALENA_HOST_CONFIG_dtoverlay #65

Closed djgood closed 2 years ago

djgood commented 2 years ago

This was an oversight on my part, since my Ribbit network had RESIN_HOST_CONFIG_dtoverlay set up by default. I decided to keep compatibility with the old variable name since anyone starting up their own network clone could run into the same problem.

This also contains a bug fix to create the balena variable with a correct name, rather than the uninitialized variable.

djgood commented 2 years ago

Meant to get this in sooner, since I really want to get our sensor up and running on the network. I accidentally broke my laptop charger and consequently the charging port for my laptop. Had to spend some time debugging and ordering a new part :)

I believe this should fix the issue. I tested running on my personal network with the same Balena variables configured as your network. No problems with the extra overlays specified, although I agree with your point from Discord that it would be useful to programmatically set up all of the device tree configuration. This allows us to be more open and also more precise since we can fine-tune configuration based on device type.

keenanjohnson commented 2 years ago

Looks good to me! Yes I created a separate issue #66 to track setting the variable programtically.

keenanjohnson commented 2 years ago

We've confirmed that this works on @djgood 's Raspberry Pi 3 with a serial GPS module.

I'm testing now on a USB GPS CM4 and will merge once verified that everything still works :)

keenanjohnson commented 2 years ago

@djgood running this on a CM4 board here, it seems that this branch has broken something as the USB Power is disabled when I run this. I'm digging into it further to see what the matter is.

djgood commented 2 years ago

@djgood running this on a CM4 board here, it seems that this branch has broken something as the USB Power is disabled when I run this. I'm digging into it further to see what the matter is.

That's weird, I'm not sure why it's effecting the USB power. My code shouldn't change the DT overlay configuration for a CM4. What is BALENA_HOST_CONFIG_dtoverlay for your CM4 board?

keenanjohnson commented 2 years ago

Yeah I’m still trying untangle exactly the issue.

The overlay needs to be dtoverlay=dwc2,dr_mode=host for the carrier but it seems like it’s getting reset. I’ll try a few things and see what I can find in the morning.

We’re you able to read out the variable via echo $var name or how did you check your variables in testing?

Thanks!

On Wed, Nov 10, 2021 at 6:53 PM Desmond Good @.***> wrote:

@djgood https://github.com/djgood running this on a CM4 board here, it seems that this branch has broken something as the USB Power is disabled when I run this. I'm digging into it further to see what the matter is.

That's weird, I'm not sure why it's effecting the USB power. My code shouldn't change the DT overlay configuration for a CM4. What is BALENA_HOST_CONFIG_dtoverlay for your CM4 board?

— You are receiving this because your review was requested.

Reply to this email directly, view it on GitHub https://github.com/Ribbit-Network/ribbit-network-frog-sensor/pull/65#issuecomment-965943349, or unsubscribe https://github.com/notifications/unsubscribe-auth/AATQ3FQ74BCQR7PUWSYLXB3ULMVZNANCNFSM5HW4U37A .

djgood commented 2 years ago

The problem probably has to do with how I tried to get fancy and disable the UART if a CDC device is detected at gpsd.py:150. It should just be connecting to the SDK, seeing that there is no disable-bt overlay set and returning.

If it's not returning, something that may be breaking things is that my code currently parses dwc2,dr_mode=host into "dwc2","dr_mode=host","disable-bt" when it adds in the disable-bt overlay. I didn't realize that dt overlays can have a comma in them. Right now, you can get around this by surrounding your dt overlay with quotes, but I'll push a fix for that.

I was checking the value of BALENA_HOST_CONFIG_dtoverlay through "Device Configuration" on the Balena dashboard.

keenanjohnson commented 2 years ago

Here's the logs from a CM4 Test device. From the container I see:

 gpsd  USB CDC device detected.
 gpsd  Settings file not found or not in proper format. Rewriting default settings to: /root/.balena/balena.cfg
 gpsd  Error creating dtoverlay
 gpsd  UART0 disabled.
 gpsd  Starting gpsd attached to /dev/ttyACM0...

then I see

Applying boot config: {"dtoverlay":["disable-bt"],"arm_64bit":"1","avoid_warnings":"1","disable_splash":"1","dtparam":["i2c_arm=on","spi=on","audio=on"],"gpu_mem":"16"}

So you can see it's definitely removing the "dwc2","dr_mode=host"

djgood commented 2 years ago

Ok, yeah, that’s clearly an issue. I’ll go back and test the USB CDC case on my set up again to figure out why it’s overwriting the overlay.

On Nov 11, 2021, at 12:39 PM, Keenan Johnson @.***> wrote:

 Here's the logs from a CM4 Test device. From the container I see:

gpsd USB CDC device detected. gpsd Settings file not found or not in proper format. Rewriting default settings to: /root/.balena/balena.cfg gpsd Error creating dtoverlay gpsd UART0 disabled. gpsd Starting gpsd attached to /dev/ttyACM0... then I see

Applying boot config: {"dtoverlay":["disable-bt"],"arm_64bit":"1","avoid_warnings":"1","disable_splash":"1","dtparam":["i2c_arm=on","spi=on","audio=on"],"gpu_mem":"16"}

So you can see it's definitely removing the "dwc2","dr_mode=host"

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

keenanjohnson commented 2 years ago

Thanks! Yeah I have a few other things I need to sort out today but I can help take a look at the source more directly sometime in the next few days to see exactly what the error is.

djgood commented 2 years ago

I'm not able to reproduce it in my fleet, unfortunately.

But my current theory for the log you posted is that the existing BALENA_HOST_CONFIG_dtoverlay value was empty or otherwise considered "false" in Python. It might have cleared itself on a previous boot, so we might only be seeing a symptom of the issue.

I've uploaded some code to hopefully make the config variable detection a bit more reliable and also parse the dt_overlay variable more carefully.

When you have a moment, can you try pushing my branch to your device? Try resetting BALENA_HOST_CONFIG_dtoverlay to dwc2,dr_mode=host and watching to see what happens.

keenanjohnson commented 2 years ago

Ok I just deployed it to a test fleet here. I can give you access to the device if you would like @djgood, but it seems like the same behavior. After pushing this branch, the BALENA_HOST_CONFIG_dtoverlay is reset to just have disable-bt.

I haven't looked at the source code closely yet with this change in mind, but I will have some time tomorrow to do so.

image

djgood commented 2 years ago

Ok, thanks! Looking at it in the dashboard should be a bit easier to debug. My Balena username is gh_djgood

keenanjohnson commented 2 years ago

Ok I just added you to the test-ribbit fleet. The device 5582468ac31e723a6def4580c131f69a (mean-photo) is now running the latest from this branch. Feel free to push to that fleet all your want. The device is inside, so the GPS reception might be a bit spotty, but the connectivity issues will be obvious.

djgood commented 2 years ago

Amazing! I should be able to figure out what’s happening, will report back.

On Nov 11, 2021, at 10:53 PM, Keenan Johnson @.***> wrote:

 Ok I just added you to the test-ribbit fleet. The device 5582468ac31e723a6def4580c131f69a (mean-photo) is now running the latest from this branch. Feel free to push to that fleet all your want. The device is inside, so the GPS reception might be a bit spotty, but the connectivity issues will be obvious.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

keenanjohnson commented 2 years ago

Awesome! Thank you so much for tackling this!

On Thu, Nov 11, 2021 at 8:08 PM Desmond Good @.***> wrote:

Amazing! I should be able to figure out what’s happening, will report back.

On Nov 11, 2021, at 10:53 PM, Keenan Johnson @.***> wrote:

 Ok I just added you to the test-ribbit fleet. The device 5582468ac31e723a6def4580c131f69a (mean-photo) is now running the latest from this branch. Feel free to push to that fleet all your want. The device is inside, so the GPS reception might be a bit spotty, but the connectivity issues will be obvious.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

— You are receiving this because your review was requested.

Reply to this email directly, view it on GitHub https://github.com/Ribbit-Network/ribbit-network-frog-sensor/pull/65#issuecomment-966804116, or unsubscribe https://github.com/notifications/unsubscribe-auth/AATQ3FXMOFRGNWTI3ENL263ULSHKJANCNFSM5HW4U37A .

djgood commented 2 years ago

I reset the variable to the override value and it seems to be working ok.

Maybe my fix fixed it? Or it’s a problem with initial boot up.

On Nov 11, 2021, at 11:18 PM, Keenan Johnson @.***> wrote:

 Awesome! Thank you so much for tackling this!

On Thu, Nov 11, 2021 at 8:08 PM Desmond Good @.***> wrote:

Amazing! I should be able to figure out what’s happening, will report back.

On Nov 11, 2021, at 10:53 PM, Keenan Johnson @.***> wrote:

 Ok I just added you to the test-ribbit fleet. The device 5582468ac31e723a6def4580c131f69a (mean-photo) is now running the latest from this branch. Feel free to push to that fleet all your want. The device is inside, so the GPS reception might be a bit spotty, but the connectivity issues will be obvious.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

— You are receiving this because your review was requested.

Reply to this email directly, view it on GitHub https://github.com/Ribbit-Network/ribbit-network-frog-sensor/pull/65#issuecomment-966804116, or unsubscribe https://github.com/notifications/unsubscribe-auth/AATQ3FXMOFRGNWTI3ENL263ULSHKJANCNFSM5HW4U37A .

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

keenanjohnson commented 2 years ago

Oh interesting. Have you tried rebooting the device? Does your overridden value stay in that case? Yeah it must be either something that only occurs during a software update or a reboot.

On Thu, Nov 11, 2021 at 8:34 PM Desmond Good @.***> wrote:

I reset the variable to the override value and it seems to be working ok.

Maybe my fix fixed it? Or it’s a problem with initial boot up.

On Nov 11, 2021, at 11:18 PM, Keenan Johnson @.***> wrote:

 Awesome! Thank you so much for tackling this!

On Thu, Nov 11, 2021 at 8:08 PM Desmond Good @.***> wrote:

Amazing! I should be able to figure out what’s happening, will report back.

On Nov 11, 2021, at 10:53 PM, Keenan Johnson @.***> wrote:

 Ok I just added you to the test-ribbit fleet. The device 5582468ac31e723a6def4580c131f69a (mean-photo) is now running the latest from this branch. Feel free to push to that fleet all your want. The device is inside, so the GPS reception might be a bit spotty, but the connectivity issues will be obvious.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

— You are receiving this because your review was requested.

Reply to this email directly, view it on GitHub < https://github.com/Ribbit-Network/ribbit-network-frog-sensor/pull/65#issuecomment-966804116 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AATQ3FXMOFRGNWTI3ENL263ULSHKJANCNFSM5HW4U37A

.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

— You are receiving this because your review was requested.

Reply to this email directly, view it on GitHub https://github.com/Ribbit-Network/ribbit-network-frog-sensor/pull/65#issuecomment-966813528, or unsubscribe https://github.com/notifications/unsubscribe-auth/AATQ3FR6POZZRUA2V5ZUJ5TULSKMJANCNFSM5HW4U37A .

djgood commented 2 years ago

Yeah, the right value persists through a reboot. I’ll try updating the device and seeing what happens.

On Nov 11, 2021, at 11:43 PM, Keenan Johnson @.***> wrote:

 Oh interesting. Have you tried rebooting the device? Does your overridden value stay in that case? Yeah it must be either something that only occurs during a software update or a reboot.

On Thu, Nov 11, 2021 at 8:34 PM Desmond Good @.***> wrote:

I reset the variable to the override value and it seems to be working ok.

Maybe my fix fixed it? Or it’s a problem with initial boot up.

On Nov 11, 2021, at 11:18 PM, Keenan Johnson @.***> wrote:

 Awesome! Thank you so much for tackling this!

On Thu, Nov 11, 2021 at 8:08 PM Desmond Good @.***> wrote:

Amazing! I should be able to figure out what’s happening, will report back.

On Nov 11, 2021, at 10:53 PM, Keenan Johnson @.***> wrote:

 Ok I just added you to the test-ribbit fleet. The device 5582468ac31e723a6def4580c131f69a (mean-photo) is now running the latest from this branch. Feel free to push to that fleet all your want. The device is inside, so the GPS reception might be a bit spotty, but the connectivity issues will be obvious.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

— You are receiving this because your review was requested.

Reply to this email directly, view it on GitHub < https://github.com/Ribbit-Network/ribbit-network-frog-sensor/pull/65#issuecomment-966804116 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AATQ3FXMOFRGNWTI3ENL263ULSHKJANCNFSM5HW4U37A

.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

— You are receiving this because your review was requested.

Reply to this email directly, view it on GitHub https://github.com/Ribbit-Network/ribbit-network-frog-sensor/pull/65#issuecomment-966813528, or unsubscribe https://github.com/notifications/unsubscribe-auth/AATQ3FR6POZZRUA2V5ZUJ5TULSKMJANCNFSM5HW4U37A .

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

keenanjohnson commented 2 years ago

I wonder if there is some difference in the way the Balena supervisor handles updates.

In my case I was applying the override at the fleet level originally. Then when the script initially sets the variable to bt_disable I would just delete the override in the Balena UI assuming it would force a reset.

However it looks like you went in and positively asserted the right overlay paramaeters at the device override level, which must be subtly different than what I did. Interesting.

On Thu, Nov 11, 2021 at 8:52 PM Desmond Good @.***> wrote:

Yeah, the right value persists through a reboot. I’ll try updating the device and seeing what happens.

On Nov 11, 2021, at 11:43 PM, Keenan Johnson @.***> wrote:

 Oh interesting. Have you tried rebooting the device? Does your overridden value stay in that case? Yeah it must be either something that only occurs during a software update or a reboot.

On Thu, Nov 11, 2021 at 8:34 PM Desmond Good @.***> wrote:

I reset the variable to the override value and it seems to be working ok.

Maybe my fix fixed it? Or it’s a problem with initial boot up.

On Nov 11, 2021, at 11:18 PM, Keenan Johnson @.***> wrote:

 Awesome! Thank you so much for tackling this!

On Thu, Nov 11, 2021 at 8:08 PM Desmond Good @.***> wrote:

Amazing! I should be able to figure out what’s happening, will report back.

On Nov 11, 2021, at 10:53 PM, Keenan Johnson @.***> wrote:

 Ok I just added you to the test-ribbit fleet. The device 5582468ac31e723a6def4580c131f69a (mean-photo) is now running the latest from this branch. Feel free to push to that fleet all your want. The device is inside, so the GPS reception might be a bit spotty, but the connectivity issues will be obvious.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

— You are receiving this because your review was requested.

Reply to this email directly, view it on GitHub <

https://github.com/Ribbit-Network/ribbit-network-frog-sensor/pull/65#issuecomment-966804116

,

or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AATQ3FXMOFRGNWTI3ENL263ULSHKJANCNFSM5HW4U37A

.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

— You are receiving this because your review was requested.

Reply to this email directly, view it on GitHub < https://github.com/Ribbit-Network/ribbit-network-frog-sensor/pull/65#issuecomment-966813528 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AATQ3FR6POZZRUA2V5ZUJ5TULSKMJANCNFSM5HW4U37A

.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

— You are receiving this because your review was requested.

Reply to this email directly, view it on GitHub https://github.com/Ribbit-Network/ribbit-network-frog-sensor/pull/65#issuecomment-966819422, or unsubscribe https://github.com/notifications/unsubscribe-auth/AATQ3FT2BEETMSM2FMKX7OLULSMQFANCNFSM5HW4U37A .

djgood commented 2 years ago

My code doesn't detect that a variable exists when the device supervisor is inheriting the fleet override and does its own creation of the variable. So fleet overwrites behave a bit different than variables. I probably have to access another API to find out if there are any overrides specified for a given variable.

Another problem is that we always create the configuration to enable the UART if the configuration variable doesn't exist. So even when it's trying to disable the UART, it will create a new variable with "disable-bt". Easy enough fix for this though.

keenanjohnson commented 2 years ago

@all-contributors please add @djgood for his great ideas and code contributions to this project.

allcontributors[bot] commented 2 years ago

@keenanjohnson

I've put up a pull request to add @djgood! :tada:

djgood commented 2 years ago

Thanks for the shout out! Glad to be helping out with such a cool project! 😄

I did some poking around to see what calls the balena dashboard is using and found the API that I need to check if a fleet-wide variable is defined. Turns out I need to first check application_config_variable to see if it's defined fleet wide and then check out device_config_variable to see what the individual device configuration is. Fix is in progress now and hopefully should resolve our issues.

keenanjohnson commented 2 years ago

This is ready to go pending review right @djgood ?

djgood commented 2 years ago

This is ready to go pending review right @djgood ?

Yup, everything should be working now!