Non-Linux Dev Envs#194
Non-Linux Dev Envs#194DXCanas wants to merge 11 commits intoJaapvanEkris:0.9.7-(under-construction)from
Conversation
…y with graceful fallbacks Changed BleManager to lazy-load native modules only on Linux platform and throw descriptive error on unsupported platforms. Updated PeripheralManager error handling to downgrade BLE initialization errors from error to warn level and automatically disable BLE/HRM modes when unavailable. Made GPIO timer service conditional on Linux platform in server.js with warning message for other platforms.
There was a problem hiding this comment.
Thanks, actually this is a good idea, and it would remove a lot of pain from me too. But I would also add a config option that disables these in case of linux.
Reason is:
- I sometimes do simulate on windows (i comment out everything that does not exist on and run things bare bones). you solution works for this
- I sometimes develop on a headless linux server (not the pi) which is linux but does not have Bluetooth (well it does but its off obviously) and does not have GPIO
What do you think?
Abasz
left a comment
There was a problem hiding this comment.
Just some general ideas on the code
|
If it helps people, let's do this. Some practical points:
|
Introduces the `simulateWithoutHardware` flag to the default configuration and config validation. This enables developers to bypass GPIO and BLE hardware checks to simulate the environment on headless servers or non-Linux systems without causing error logs. AI-assisted-by: Gemini 3.1 Pro
Removes the static `process.platform === 'linux'` check in favor of dynamically importing `hci-socket` and `ble-host`. Uses the new `simulateWithoutHardware` config flag to bypass BLE initialization when requested. Refactors `open()` to use guard clauses, preventing deep nesting and improving readability. Catches and logs missing dependencies cleanly. AI-assisted-by: Gemini 3.1 Pro
Removes the static `process.platform === 'linux'` check from `server.js` and moves hardware checking into `GpioTimerService.js`. Uses dynamic imports for `pigpio` wrapped in a try/catch block. If `simulateWithoutHardware` is true or if `pigpio` fails to load (e.g., on non-Raspberry Pi devices or without root), the service logs a warning and exits cleanly instead of crashing the app. AI-assisted-by: Gemini 3.1 Pro
DXCanas
left a comment
There was a problem hiding this comment.
Isolate this stuff well and log what conclusion is drawn (so start the modules with the if...then...else statement which always logs), to make sure that we can detect problems from logging
Tried to make sure the logs robust enough @JaapvanEkris
Tested; server start on non-compatible platform is still a little noisy because we log the errors, but everything spins up fine.
| let pigpio | ||
| try { | ||
| const pigpioModule = await import('pigpio') | ||
| pigpio = pigpioModule.default | ||
| } catch (error) { | ||
| log.warn('Failed to load pigpio module. GPIO service will be unavailable (Linux/Raspberry Pi only).', error.message) | ||
| return | ||
| } | ||
|
|
There was a problem hiding this comment.
Is this robust enough without having to look for specific Pi model names @JaapvanEkris ?
There was a problem hiding this comment.
No, it's not. Digging some more.
There was a problem hiding this comment.
I ended up going with a caught error and message:
The failure message is annoying, but the JS layer seems to obfuscate any compatability checks at the C-library level.
So this is what we get:
Server: peripheral requested refreshPeripheralConfig
webserver running on port 80
+-----------------------------------------------------------------------+
| Warning: The pigpio C library can't be loaded on this machine and any |
| attempt to use it will fail. |
| |
| Error: "Module did not self-register" |
| ------------------------------------ |
| If you are working on a Raspberry Pi and see a "Module did not self- |
| register" error, this typically indicates that the installation |
| instructions were not exactly followed. For further details see the |
| installation section of the readme at |
| https://github.com/fivdi/pigpio#installation |
| Note that step 1 of the installation instructions must be completed |
| before step 2. |
+-----------------------------------------------------------------------+
Invoking require('bindings')('pigpio.node') resulted in the follwoing error:
Error: Module did not self-register: '/Users/david/Projects/openrowingmonitor/node_modules/pigpio/build/Release/pigpio.node'.
at Module._extensions..node (node:internal/modules/cjs/loader:1920:18)
at Module.load (node:internal/modules/cjs/loader:1481:32)
at Module._load (node:internal/modules/cjs/loader:1300:12)
at TracingChannel.traceSync (node:diagnostics_channel:328:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:245:24)
at Module.require (node:internal/modules/cjs/loader:1504:12)
at require (node:internal/modules/helpers:152:16)
at bindings (/Users/david/Projects/openrowingmonitor/node_modules/bindings/bindings.js:112:48)
at /Users/david/Projects/openrowingmonitor/node_modules/pigpio/pigpio.js:8:31
at Object.<anonymous> (/Users/david/Projects/openrowingmonitor/node_modules/pigpio/pigpio.js:43:3)
-------------------------------------------------------------------------------
Hardware initialization: pigpio C library failed to initialize (likely not a supported Raspberry Pi or missing root). GPIO service is bypassed.
Which. I mean. Not great, but it works, and if someone's using not root they still get the scary messages.
I did explicitly check for the devices mentioned, but thought it was a bit brittle?
893a01e
Updates `GpioTimerService` to explicitly read `/proc/device-tree/model` to confirm it is running on a supported Raspberry Pi (3, 4, or Zero 2W) before attempting to load `pigpio`. Updates `BleManager` to explicitly check for Linux before attempting to load `hci-socket`. Both services now log explicit messages detailing their initialization decisions as requested by maintainers, ensuring clear visibility into why hardware modules are bypassed or loaded. AI-assisted-by: Gemini 3.1 Pro
cfca9fe to
e87451b
Compare
…on failure gracefully The `pigpio` JS wrapper exports functions even if the underlying C library fails to initialize (e.g. on macOS or non-Pi Linux without root), causing a hard crash later when those undefined C-bindings are called. This commit explicitly wraps the first call to `pigpio.hardwareRevision()` in a try/catch block to catch the `TypeError: is not a function`. This prevents the application from crashing and instead cleanly bypasses the GPIO service as requested by maintainers. AI-assisted-by: Gemini 3.1 Pro
e87451b to
fcabffd
Compare
Abasz
left a comment
There was a problem hiding this comment.
Just some general ideas on the code
… JSDoc imports Replaces the temporary `any` types with `typeof import(...)` for the BLE modules (`hci-socket` and `ble-host`). This restores strong type checking for the class properties while keeping the actual module loading deferred to runtime, satisfying the requirement to lazy-load these modules only on supported platforms (Linux) without losing static analysis benefits.
This reverts commit 5a352e5.
|
See also #165 |
|
I see this is slated for 0.9.8, which is fine! But just in case the state of the PR is ambiguous… this one is Ready the only reason I didn't resolve the comment above is because I wanted @JaapvanEkris to have a chance to object to the approach taken, if you see fit 🙂 |
|
I have been trying to use this so I can develope in a github codespace. I have set the simulateWithoutHardware to true, but the initialization still fails. I think the issue is that we reject the promise and that is never cought at any level at the callstack. if (config.simulateWithoutHardware) {
log.info('Hardware initialization: simulateWithoutHardware is true. BLE manager is bypassed.')
return Promise.reject(new Error('simulateWithoutHardware is true, BLE disabled.'))
}Uncaught Exception: Error: simulateWithoutHardware is true, BLE disabled.
at BleManager.open (file:///workspaces/openrowingmonitor/app/peripherals/ble/BleManager.js:35:29)
at BleManager.getManager (file:///workspaces/openrowingmonitor/app/peripherals/ble/BleManager.js:107:17)
at setup (file:///workspaces/openrowingmonitor/app/peripherals/ble/FtmsPeripheral.js:78:33)
at createFtmsPeripheral (file:///workspaces/openrowingmonitor/app/peripherals/ble/FtmsPeripheral.js:75:3)
at createBlePeripheral (file:///workspaces/openrowingmonitor/app/peripherals/PeripheralManager.js:242:25)
at setupPeripherals (file:///workspaces/openrowingmonitor/app/peripherals/PeripheralManager.js:115:11)When you were testing this, what was the setup? Did this work for you? EDIT: |
I always turn BLE off en comment out GPIO. And then it all works. Never been an issue for me, |
|
Hm. I've only ever used it with |
|
Stupid question, have a change coming in but not able to test tonight |
…T+ initialization attempts Relocates the `simulateWithoutHardware` check from individual BLE peripheral setup() functions to the `createBlePeripheral()` and `createAntPeripheral()` functions in PeripheralManager. This prevents BLE and ANT+ peripherals from being created at all when hardware simulation is enabled, rather than catching rejected promises after initialization has already started. Removes now-unnecessary .catch() handlers
|
Sorry about that one. I pulled the rug underneath this one by removing the 0.9.7 branch after merging it into main. My bad. |
|
This was ready anyways actually :D |
|
I just realized that this was not merged after not understanding why things are not working on github codespace :D Actually this worked very well for me. Can we have this back please? |
|
I pushed this to 0.9.8 (hence its label) as I already had enough loose ends to manage during the last integration (lots of small bug fixes at the last moment, one of my/areas of improvement). As we didn't intend to do any major work in the tail of 0.9.7, I figured it would be the first push onto the 0.9.8 codebase. |
Yes no problem, it was my mistake that I did not see that this was not merged but closed actually. What I was trying to say is that we should definitely have this @DXCanas and I would actually merge it rather early for 0.9.8 as it is a tremendous help for the client development :D |
Hey guys. Last one for tonight, promise.
So. My current laptop is a mac, though I manage linux servers for work. Go figure.
But I have been replaying raw data to test GUI updates. And the server will not start without linux env checks.
This is Claude's attempt at catching those errors, so that I can work on the project. I am tired of merging and un-merging this into candidate branches :)