Draft: micropython/lora: Add async-capable LoRA modem driver
Still relatively early, but starting to be useful for things.
Eventual goal is to have enough support for MicroPython to be a LoRaWAN Class A device with a fully async API available.
TODOs
(In addition to anything flagged "TODO:" in the code)
- [ ] Confirm the API & AsyncModem wrapper design is a reasonable approach
- [ ] Support ThreadSafeFlag.clear() in MP
- [ ] Figure out why the RxTimeout interrupt line is toggling during TX (currently using
_irq_enas a workaround) - [x] Split out the single configure() function to allow fast runtime configuration of parameters changed on the fly by the LoRAWAN MAC layer. (Done by having configure() keep the current value for any missing keys, instead of setting defaults.)
- [ ] Decide about support for interleaving rx,tx as managed by the driver - i.e. it would be nice if one task could be continuously (or arbitrarily) calling
receive()while another task sometimes calledtransmit(), and have the modem driver handle the state changes internally without the caller needing to do anything. This isn't necessary for LoRAWAN, but it would make some other types of application logic much simpler. - [ ] Decide about support for
rx_continuousmode. If the previous dot point can be implemented then it might not be needed to explicitly support putting the modem into continuous RX mode. - [ ] Review use of
assertin the code - some places are probably ValueErrors, some places may be unnecessary. - [x] Add support for a second modem apart from SX1276. (Done, STM32WL5x.)
- [ ] Resolve issue where SX1276 invert_iq_tx setting seems to be inverted from all other modems (this may be a hardware compatibility issue, but it's unusual!)
- [ ] Documentation
Example API usage
See https://gist.github.com/projectgus/aa050dd75ecfddeb6c2bfcd82a36bd6d
Hello @projectgus
Great. This is a great lib for the MicroPython system!
- This lib support all features of the SX1276?
- This lib is really specific to the SX1276 or support other models?
Thank you very much.
Sorry, I see above now that is specific for the SX1276.
I was questioning that because I would like to use in the feature the 2.4Ghz LORA https://www.semtech.com/products/wireless-rf/lora-24ghz for indoor and outdoor location - It has ~3 meters accuracy, like as the SX1280
@beyonlo The idea is to have a generic LoRA modem API that can be extended to other modem models in the future, such as the SX1280. However at least for now the focus is on messaging and supporting LoRAWAN nodes, rather than ToF range measurement.
@projectgus No problem. To have a Micropython driver for messaging on LORA nodes, will be great :)
I've had a play with this driver using 2x SX1276 modules connected to a PYBLITEv1.0 and a PYBD-SF2. Then they run the rxtx_sync.py demo code. I didn't wire up the D0/D1 or RST pins, so it's just using SPI and CS.
It works! Both devices can send and receive to/from each other.
For the power consumption, it seems quite high. I measure about 13mA when in RX mode (on top of the pyboard current), and about 30-45mA when transmitting a message (again, on top of pyboard current). Is that expected?
Decide about support for interleaving rx,tx as managed by the driver - i.e. it would be nice if one task could be continuously (or arbitrarily) calling
receive()while another task sometimes calledtransmit(), and have the modem driver handle the state changes internally without the caller needing to do anything.
I think this would be very useful, for the driver to automatically manage switching between TX and RX.
For LoRaWAN, or for a custom LoRa protocol, is the idea to have RX constantly on (except when transmitting), or for RX only to be active periodically to reduce power consumption?
For the power consumption, it seems quite high. I measure about 13mA when in RX mode (on top of the pyboard current), and about 30-45mA when transmitting a message (again, on top of pyboard current). Is that expected?
That sounded high to me also, but SX1276 datasheet says "Supply current in receive mode" between 10.8 and 11.5mA for AU915 depending on which antenna is used. Transmit current 20-120mA depending on transmit power level.
So probably to be expected.
Decide about support for interleaving rx,tx as managed by the driver - i.e. it would be nice if one task could be continuously (or arbitrarily) calling receive() while another task sometimes called transmit(), and have the modem driver handle the state changes internally without the caller needing to do anything. For LoRaWAN, or for a custom LoRa protocol, is the idea to have RX constantly on (except when transmitting), or for RX only to be active periodically to reduce power consumption?
LoRaWAN specifies particular time slices when the node has to receive.
Given how much of a power drain constant receiving is, it won't be desirable for battery-powered devices. It's likely most custom LoRa setups will have at least one end on battery power, so it's probably not that useful to have seamless RX/TX switching in the driver. Will pull it out.
So probably to be expected.
Yeah, I came to the same conclusion reading the SX datasheet.
Will pull it out.
OK. We can always put it back in when we have more experience with this driver. For now, best to keep it simple/minimal.
@dpgeorge Thanks for the review, I've updated based on your comments and did some additional memory usage optimisations (especially in sx1262.py).
The stm32wl5 support has now been split into an sx1262.py driver, tested with a local module, and I've moved the small amount of additional changes needed for the stm32wl5 into another branch for a separate PR. This means this PR no longer requires any MicroPython changes, and @jimmo your modules should now work properly with it!
Looking good!
One thing that's needed is a manifest.py for packaging. Also to consider how to organise the files in this package, and whether they should be split out into separate packages or components within the one package. For example, as a user I probably only want either the SX1276 or SX1262 driver, but not both. And I probably only want to use AsyncModem or Modem, but not both.
Probably the directory micropython/lora should have subdirs in it for the actual packages, like micropython/lora/sx1276 and micropython/lora/lora-modem etc. See aioble for a good example of how to structure things. @jimmo will probably have opinions here :smile:
@dpgeorge Can do! Sorry, I did know I needed to do this but I hadn't put it on the TODO list. Will crib from aioble and come up with something soon.
Modem is actually an abstract base class, so everyone will need that[*] - I will rename it to BaseModem or something and make sure everything is laid out to make this clearer too.
[*] It does mostly implement the synchronous methods that using the AbstractModem() wrapper removes the need for, but I can't think of a better hierarchy that doesn't include multiple inheritance. And I think probably best to avoid multiple inheritance!
Hey @tve,
Thanks for taking a detailed look through this. I appreciate that you seem to be pretty familiar with these radios, it's great to get that perspective as I'm no LoRa expert at this stage.
May defer a few of the in-code replies until I have a chance to check properly on the answers.
break all sub-parts of config() out into separate functions so they can be called when needed, examples are changing BW/SF and TX power in response to ACKs (or lack thereof) or changing channels (freq) (see https://github.com/lemariva/uPyLoRaWAN/blob/master/sx127x.py for example where the init is a collection of calls to these broken out functions)
This was modelled after how the micropython-lib bluetooth config() function works, with the idea that any keys which aren't set in the argument dict are kept at their current values. So it's possible for one call to either change one or two parameters, or set all the parameters, depending on the arguments.
I believe this makes for smaller code size, but I could have that wrong. @dpgeorge @jimmo is that right, and is this still a good approach?
ability to have continuous RX as the default mode as opposed to switching back to standby (that's what I tend to use for powered devices) provide functionality to check for RX in-progress before TX
The first version of this PR had some of this, but I dropped it as I wasn't sure how useful it was, and it adds a bunch of complexity to manage the concurrency around resuming RX cleanly. Will look at adding it back.
My thought was this can be done in this simplified driver by calling receive() in a loop with a relevant timeout, and if also transmitting then call transmit() in between calls to receive() as needed. I was mostly thinking from the point of view of either fairly simple simplex schemes, or LoRaWAN nodes which have specific short receive and transmit windows but the radio is otherwise idle and window timing is controlled by the host.
If you have a different use case with more complex transmit/receive windows and timing that you'd like this to support the please post more details - I'd like to properly understand it.
provide a function to calculate packet duration (with LoRa it's not that trivial and often needed since air time can be quite long) provide functions to enter and leave sleep mode, incl. setting all pins to high impedance provide a function to read the FEI (frequency error indication), I find it pretty useful to tune the frequency of sensor nodes when using FSK mode; I don't know whether LoRa is more robust WRT freq deviation (hmmm, certainly not in the very narrow band modes) and the FEI is an easy way to ensure the nodes agree with a central gateway (I'm not using LoRaWAN)
All good suggestions, will do!
I find the way the IRQ flags are managed rather fragile. All the masking/unmasking only works if start_xx/end_xx are always correctly called in the right sequence. If they're not, one is guaranteed odd and difficult to troubleshoot results. I'm used to clear all flags before switching to TX/RX mode so I'm guaranteed I'm starting from a known state for that phase. Also, I've never used the IRQ masks on the semtech chips and rather do a dispatch in the IRQ handler to figure out what happened. You seem to experience some spurious interrupt according tot he comments and I wouldn't be surprised if it had to do with changing mode, changing IRQ masks, and changing DIO Mapping in back-to-back SPI transactions. There's a wimpy little uC in that sx1276 that has to keep up and do the right thing kepping in mind that the > mode change may take way longer than the subsequent SPI transactions (we know that little uC has bugs...)
Goal here was mostly to keep the code small as possible, but this is a reasonable point - especially if it does turn out that masking is causing some of the spurious interrupt issue.
I really only anticipated modem users calling transmit()/receive() directly (sync or async). The start_xx/end_xx /is_receive_finished/is_transmit_finished are primarily an "internal" API for each modem chip. Of course in that case they should be prefixed as such, will change that.
I know there are use cases where the desire is to call start_xyz and end_xyz manually and interleave other computation in between. My feeling was that switching to async and await m.transmit() or await m.receive(timeout) instead will make the whole thing simpler.
WRT polling for RX I would not provide any examples or high level code that polls the radio for RX complete. It's a noise disaster in my experience. (Maybe different in stm32wle.) I believe the only appropriate non-interrupt usage pattern for > RX is to have an RX window, start the RX at the beginning of the window, wait, and then check whether something arrived once at the end of the window.
Interesting, I agree supporting non-interrupt completion is definitely a sub-optimal fallback. Can you elaborate on "noise disaster"? Do you mean you get significant RF noise injected from the SPI transactions?
I did not see high-level code to operate the driver in always-RX mode and I believe it's rather tricky. Specifically because the IRQ handler gets no info about why it got called and there's an issue with calling RX when no RX-complete indication has been provided (e.g. because one wants to TX).
Agree this is not really possible in this version, see the answer above. If you've got some examples of exactly how you'd like to use the driver, that's probably the best way for me to understand what you'd like it to support internally.
Documentation???
Yes, definitely. This is a Draft PR and there is a TODO checklist in the first post which includes an item for documentation. I have them half-written now but I wanted to get feedback on the API and approach as early as possible. So thanks for providing good feedback on those things here, it might put the actual docs back a little bit though(!) depending if any changes are made.
Re: not breaking config into several smaller functions: I can work with that. Your code does 5-6 unnecessary SPI read-modify-write cycles if given an empty config map, though. That's not so cool. Also, I wonder how long triggering all these try-except blocks takes, plus getting the chip out of sleep mode again.
If you have a different use case with more complex transmit/receive windows and timing that you'd like this to support the please post more details - I'd like to properly understand it.
Two use-cases:
- I have lots of sensors around the property and have several gateways. The gateways are powered and relay between LoRa and WiFi, they consist of an esp32 + sx1276. The gateways always have RX enabled. When they receive a packet they briefly turn TX on one second later for an ACK. Also, when they receive an outbound packet over WiFi they briefly switch to TX to forward that packet on LoRa.
- I'm just completing a "buddy tracker" that consists of two nodes, each with GPS, and each displaying an arrow pointing in the direction of the other node. Use-case is paddling in the open ocean where it's tough to keep track of where someone else is when the waves are overhead. These nodes TX every second and self-synchronize so they alternate their TX every half second. I'm hoping to extend this to more than two nodes. RX is always on (except for TX) 'cause managing to get an RX is way more important than battery, which has a hefty load due to the display and GPS anyway. In this use-case I could calculate the RX duration such that it ends right when the node needs to TX, but that complicates the app code.
In general, if the power budget is not an issue then why wouldn't you want to have RX always on?
I'm used to clear all flags before switching to TX/RX mode so I'm guaranteed I'm starting from a known state for that phase. Goal here was mostly to keep the code small as possible, but this is a reasonable point - especially if it does turn out that masking is causing some of the spurious interrupt issue.
I believe that just writing 0xff to the IRQ flags and not dealing with the mask at all actually takes less code.
I know there are use cases where the desire is to call start_xyz and end_xyz manually and interleave other computation in between. My feeling was that switching to async and
await m.transmit()orawait m.receive(timeout)instead will make the whole thing simpler.
Please provide an async code example for running in continuous RX with async events causing a need to TX.
Do you mean you get significant RF noise injected from the SPI transactions?
Yes! I see a big difference in RX sensitivity once you get close to the noise floor if you're constantly polling the IRQ flags. This was a while ago but I know it was on a PCB, i.e., not breadboard with wires.
I did not see high-level code to operate the driver in always-RX mode and I believe it's rather tricky. Specifically because the IRQ handler gets no info about why it got called and there's an issue with calling RX when no RX-complete indication has been provided (e.g. because one wants to TX).
Agree this is not really possible in this version, see the answer above. If you've got some examples of exactly how you'd like to use the driver, that's probably the best way for me to understand what you'd like it to support internally.
I provided some use-cases. For me, this is a complete deal-breaker, i.e., fork&modify situation. Also, I would caution against assuming too many chip-internal states implicitly, like "if this flag is not set it must be that other one and I don't need to check it". In my experience this comes to bite later when misbehavior is discovered.
As an example, I recently saw https://forum.lora-developers.semtech.com/t/sx1262-reduced-rx-sensitivity-packet-reception-fails/162. The failure hypothesis is that AFC/AGC gets off-track due to a strong nearby interference and then the chip misses weak packets that it would normally RX because it's stuck off-frequency and/or low gain. I don't know whether this is what is happening, but I know that a naive driver for sx1231 (rfm69) or sx1276 in FSK mode looks completely different from a good driver, because a good driver has to "watchdog" the chip and reset it if there's any indication that it's stuck. This entails getting interrupts on detecting a high RSSI level, setting a timer, and restarting RX if the chip is still stuck in trying to decode that non-existent packet at the timeout. It has now been so many years that I (and other people) have fought this issue without any ACK from semtech that I wouldn't be surprised if the LoRa chips had that "signature feature" as well. I had hoped to avoid that in LoRa and my heart sank when I bumped into that thread. I haven't confirmed this behavior myself, so I'm secretly hoping there is some other benign issue...
Documentation???
Yes, definitely. This is a Draft PR and there is a TODO checklist in the first post which includes an item for documentation. I have them half-written now but I wanted to get feedback on the API and approach as early as possible. So thanks for providing good feedback on those things here, it might put the actual docs back a little bit though(!) depending if any changes are made.
Ah, I had missed that checkbox, sorry for that. I actually find that writing the docs first is extremely helpful. I often end up with a paragraph explaining something and then realize that I should really change the API so the explanation turns into a short sentence.
Looking forward to the next iteration!
Re: RX getting stuck . Notice the sentence: "In the GFSK Rx continuous mode on the SX1268 chip and in the LoRa Rx continuous mode on the SX1276 chip, the problem does not appear." in go.valeriy's post.
Some other choice posts in the forum:
- https://forum.lora-developers.semtech.com/t/sx126x-strange-behavior-after-rx-timeout/1080/3
- https://forum.lora-developers.semtech.com/t/new-sx1276-chip-revision-with-reg-version-0x13/588
- https://forum.lora-developers.semtech.com/t/sx1280-settxparams-and-setsleep/535 (that's sx1280, so not yet relevant, but relevant to the way the config() API works)
- https://forum.lora-developers.semtech.com/t/sx1276-misses-received-lora-packets/436 (no second person confirming the issue)
If you want a headache just browse the forum :-O
The upshot of all this, again, just personal experience, is that as these chip features surface the code gets more complicated and nifty code-saving assumptions no longer hold...
I really only anticipated modem users calling transmit()/receive() directly (sync or async). The start_xx/end_xx /is_receive_finished/is_transmit_finished are primarily an "internal" API for each modem chip. Of course in that case they should be prefixed as such, will change that.
I know there are use cases where the desire is to call start_xyz and end_xyz manually and interleave other computation in between. My feeling was that switching to async and await m.transmit() or await m.receive(timeout) instead will make the whole thing simpler.
As you ponder the API, please consider the following, none of which are likely to be satisfied by AsyncModem I assume, perhaps incorrectly:
- I need to get an as precise as possible timestamp for packet RX so I can synchronize nodes, I'm assuming this means I need to write my own interrupt handler, thus I'll be calling the various transmit and receive functions in SX12xx.py directly and will not use Sync/AsyncModem.
- Without using uasyncio for simple nodes I often overlap TX and RX with other stuff. This is often way simpler than throwing asyncio at the problem. Some examples:
- wake up, get sensor measurement, start TX, go back to sleep (don't bother waiting for TX-done)
- wake up, get sensor measurement, start TX, low-power-sleep for the expected TX duration, switch radio to sleep, tri-state everything and go to deep sleep
- sit in a loop consisting of: get sensor values, start TX, do some other work, wait for TX completion, start RX for ACK, do some more other work, wait for RX done/timeout
- in my tracker the loop is: TX position and wait for completion, start RX, read GPS, update display, sleep, check RX, based on RX timing sleep a small amt more to hit the correct TX timing
- I will want to collect FEI and perhaps packet CRC error stats as well, unless these are exposed this will require dealing with SX12xx.py directly and writing my own interrupt handler
- I will want to periodically query background "RSSI" to get info on the noise floor, again, mess with SX12xx.py directly and write my own high-level loop
- I may finally get around to querying temperature to gauge the need for recalibration and also to have some warning if the chip is overheating (e.g 20dBm+ operation)
Yet something else to consider... In the esp32 lightsleep and deepsleep can be interrupted by a pin level but not edge (IIUC). In order to be woken up from sleep when a packet arrives one would use DIO0 and DIO1 as wake-up pins. In order to be able to go back to sleep after handling the interrupt the pin level has to be low again (again, IIUC), this means the interrupt handler needs to clear the IRQ flag.
NB: this is another use-case for a custom interrupt handler: being able to go back to sleep.
Big update, lots of additional functionality! @tve @dpgeorge @jimmo Please take a look and let me know what you think.
Still a few small outstanding jobs, but most I've pushed to new PRs as this one is too big already. See first post for details.
The only real downside of adding a lot of functionality is adding code size:
| file | November version (95204e4) | this version (9c90a5995) |
|---|---|---|
| modem.mpy | 539 | 2542 |
| sx126x.mpy | 3599 | 5603 |
| sx127x.mpy | 3320 | 4641 |
| async_modem.mpy | 634 | 1397 |
| Total | 8092 | 14183 |
Most of this functionality are things that anyone doing non-trivial LoRa applications will find useful though, and won't be that simple to implement on top of a simpler driver. So I'm not sure I have a better suggestion to make on reducing size here.
@tve regarding a couple of these points:
I will want to collect FEI and perhaps packet CRC error stats as well, unless these are exposed this will require dealing with SX12xx.py directly and writing my own interrupt handler I will want to periodically query background "RSSI" to get info on the noise floor, again, mess with SX12xx.py directly and write my own high-level loop I may finally get around to querying temperature to gauge the need for recalibration and also to have some warning if the chip is overheating (e.g 20dBm+ operation)
There is some CRC awareness now, the driver by default drops invalid CRC packets and increments a counter crc_errors on the modem object that can be read or cleared by the programmer as needed (EDIT: now documented). There is also a field option rx_crc_error that can be set to True in order for the modem to return messages with invalid CRCs rather than drop them.
I want to add an instantaneous RSSI function as well, but this isn't done yet (I'll put it in a future PR, or contribution welcome!).
With LoRa, FEI seems to only be supported on SX1276 which is a bit surprising. All I see in the SX126x documentation is a guide to keep the bandwidth within the error limit of the clock source and an equation for that. So probably won't make this part of the common modem API, if it's not on any of the newer modems.
However, for cases like this then it should always be possible to call modem._read_reg() to read out particular register fields and do "advanced" modem-specific stuff. In particular, the packet statistic registers seem to stay valid until the next packet is received so can easily call them after a receive ends and before going back to sleep. Ditto for manually checking the temperature registers and determining if re-calibraiton is needed.
If you want a headache just browse the forum :-O
Regarding silicon bugs, this is indeed sobering reading. From what I can see, the only fully confirmed bug for this driver in that list is SX126x may get stuck on "min gain" when the receiver gets swamped temporarily with a strong signal. I think probably the best thing to do with these is to document "known" (if not confirmed) silicon bugs in the README and give some workaround tips (like stopping a continuous receive periodically if nothing is received).
I did find what seems like it might be a very nasty silicon bug though, workaround applied in b6eaf74. I still feel like it's too nasty to be that simple, but logic analyzer traces confirm the commands on the wire look correct and the modem otherwise works fine... Might report this one to Semtech as well.
Added some more updates and cleanups.
Someone also emailed me to report this driver works well on SX1278 as well (yay!) Looking at the datasheet I don't see any reason not to support all four SX127x modems, so it's been updated to do this (840b374).
One thing @jimmo and I just discussed was splitting out the sync-modem methods (namely BaseModem.transmit, BaseModem.receive and BaseModem._sync_wait) into a separate class called SyncModem. That would take a particular SX12xx instance in its constructor, essentially mirroring AsyncModem. It would live in lora-sync/lora/sync_modem.py.
Benefits:
- a clear separation of layers (BaseModem -> SX12xx -> SyncModem/AsyncModem)
- no need to have any of the sync modem code if you only use AsyncModem
Drawbacks:
- users must now always install (at least) two packages: one for their hardware, eg
sx126x, and one for the top-level driver (sync or async)
@projectgus what are your thoughts on that?
@dpgeorge
One thing @jimmo and I just discussed was splitting out the sync-modem methods (namely
BaseModem.transmit,BaseModem.receiveandBaseModem._sync_wait) into a separate class calledSyncModem. That would take a particular SX12xx instance in its constructor, essentially mirroringAsyncModem. It would live inlora-sync/lora/sync_modem.py.
@tve also asked about the possibility of a change like this, so it's clearly a logical layout for folks.
I agree about the benefits, it's a more symmetrical set of relationships. The reasons I avoided it until now was a general idea of "keep the simple case - Sync - as simple as possible", but also that I don't know how to logically describe these classes if we split them into two types of "thing":
- SyncModem & AsyncModem are one type of thing ("modem interface"?)
- SX1276, SX1262, BaseModem, etc are a different type of a thing ("modem implementation"?)
... not knowing what names to give them makes me think it's a code smell.
(I guess the current code has this problem as well regarding AsyncModem, and maybe if keeping the current structure then AsyncModem should be called AsyncAdapter or something to disambiguate... this is all starting to feel very Java-y!)
The other way around this would be to use multiple inheritance, so each "public" class in the package inherits from either SyncModem or AsyncModem and also from a chip-specific modem base (SX1276, SX1262, etc.), meaning you can do something like from lora.sync import SX1276. I generally avoid multiple inheritance (and I'm not even sure how well MP supports it) but actually this might be the most intuitive class structure to go with?
Method renames have been implemented!
One thing @jimmo and I just discussed was splitting out the sync-modem methods (namely BaseModem.transmit, BaseModem.receive and BaseModem._sync_wait) into a separate class called SyncModem. That would take a particular SX12xx instance in its constructor, essentially mirroring AsyncModem. It would live in lora-sync/lora/sync_modem.py.
@dpgeorge @jimmo Please take a look at the head commit in this branch. I'm not 100% sure this is a good idea, but I think it's still preferable to having SyncModem/AsyncModem wrappers that take chip-specific objects as arguments.
I put some reasons in the previous comment on this PR, but the final challenge was telling the user which object to call which method on. For example, do you call modem.configure(...) or modem.modem.configure(...)? The old AsyncModem wrapper had some hacky workarounds for this like def configure(self, ...): self.modem.configure(...), but this is not a great solution either.
If you think the "mixin-like class" approach in that commit is too fiddly then I fully understand though, I'm not 100% convinced on it myself - the best I could say for it is that it might be the "least bad" way. Am happy to drop thjs commit and try something else instead if you're not convinced.
(Rebased, tweaked some wording in the README a tiny bit.)
Rebased, pushed a bugfix with CRC as reported by Barry on Discord and another bug fix with async wait timeout on send (only relevant if IRQ doesn't trigger as expected).
Please make it configure the pins in the constructor (using hasattr(pin, init)). See sdcard.py for example.
Also rename u-module to module.
@dpgeorge @jimmo Thanks for the review of a large and complex PR! Have applied the requested changes (see 30214ee, 247597b), re-tested the two major modem models with sync and async code, squashed commits and rebased on master.
EDIT: Oh well, looks like when I rebased my individual fix commits were lost from GitHub. But the fixes are in there. :)
I tested the simple_rxtx code, both sync and async versions, on a pair of SX1276's. It works!
(I had to fix the simple_rxtx_async.py to rename uasyncio to asyncio, and I pushed that here.)
Thanks for the huge effort on this @projectgus !