Giter VIP home page Giter VIP logo

Comments (67)

2bndy5 avatar 2bndy5 commented on September 22, 2024 1

If we go with an abstracted API class, then we could eventually incorporate support for Texas Instruments' chips that can replicate the ESB protocol. That's not a priority though.

This might be the beginning of v2.x for the RF24 stack...

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024 1

To be fair, C++ concepts are a kind of template that shouldn't require much change in higher layer abstractions. I've never played with these because I learned C++ before the C++20 std was introduced.

Inheritance might be "cleaner" as for code clutter. My main concern is increasing compile size. It has been a while since I tried polymorphism in C++...

My initial idea was something like this
// in RF24_ABS_API.h (located in RF24 repo)

class RF24_API
{
  virtual bool begin() = 0;
};
// in RF24.h
#include "RF24_ABS_API.h"

class RF24 : public RF24_API
{
  override bool begin();
};
// in nrf_to_nrf.h
#include "RF24_ABS_API.h"

class nrf_to_nrf : public RF24_API
{
  override bool begin();
};
// in RF24Network.h
#include "RF24_ABS_API.h"

class RF24Network
{
  RF24Network(RF24_API* radio, ...);
};

from rf24network.

TMRh20 avatar TMRh20 commented on September 22, 2024 1

I think it could be improved

Hahaha, you think?

I'll have to take some time and think about this, maybe try again.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024 1

My attempt is on template-attempt2 branch. The Linux CI also indicates that I need to update the python wrapper since I adjusted the definitions for Linux as well.

I'm not sure if it is worth it since this templating tactic could be used to support other external radio HW on Linux. But, I think we can typedef (& macro sub) the template stuff out

class RF24;
#if defined RF24_LINUX
typedef ESB_RADIO RF24;
    #define ESB_RADIO_TEMPLATE_SPEC
#else
    #define ESB_RADIO_TEMPLATE_SPEC template<class ESB_RADIO>
#endif

And just use the macro ESB_RADIO_TEMPLATE_SPEC instead of the template<class ESB_RADIO> decl.

from rf24network.

TMRh20 avatar TMRh20 commented on September 22, 2024 1

Hmm, this ain't so different from what I was trying to do, except that it works! The only problem I ran into so far is that it gives linker errors when I try to use nrf_to_nrf on my XIAO BLE Sense 52840 with the MBED core The other core works fine as does the Feather 52840. Tried changing the defines to accommodate and no luck.

This is a big step towards getting this issue sorted out!

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024 1

it gives linker errors when I try to use nrf_to_nrf on my XIAO BLE Sense 52840 with the MBED core

The Seeduino MBed core uses a slightly more specific -D arg for the chip:

-DARDUINO_ARCH_NRF52840 -MMD -mcpu=cortex-m4 -mfloat-abi=softfp -mfpu=fpv4-sp-d16 -DARDUINO=10607 -DARDUINO_SEEED_XIAO_NRF52840 -DARDUINO_ARCH_MBED -DARDUINO_ARCH_MBED -DARDUINO_LIBRARY_DISCOVERY_PHASE=0

Nonetheless, it is fixed by adding to the #if defined condition:

-#ifdef ARDUINO_ARCH_NRF52
+#if defined(ARDUINO_ARCH_NRF52) || defined(ARDUINO_ARCH_NRF52840)

BTW, the nrf_to_nrf lib is flagged as incompatible with the mbed core because of the arch specified in the library.properties:

WARNING: library nrf_to_nrf claims to run on nordicnrf52, nrf52 architecture(s) and may be incompatible with your current board which runs on mbed architecture(s).

But, I don't see a adequate solution unless mbednrf52 is acceptable. I think the core blandly specifies mbed as its arch. This is probably due to the fact that all mbed arduino cores use the same repo as a foundation.

from rf24network.

TMRh20 avatar TMRh20 commented on September 22, 2024 1

Yeah, I had a quick look at RF24Ethernet yesterday, after seeing your ESBMesh and Network changes, I thought I would give RF24Ethernet a look. Didn't get very far lol.

we could even use SkyNet & SkyMesh

Hahaha, I think we would be taking our chances there. I don't want to be 'terminated'

I kind of like the ESBRadio, ESBNetwork, ESBMesh names, it more properly describes them than RF24 or NRF52 prefixes.

from rf24network.

TMRh20 avatar TMRh20 commented on September 22, 2024 1

Well if you can't figure it out, hopefully I can answer them. RF24Ethernet involved a lot of copy/paste from https://github.com/ntruchsess/arduino_uip now found at https://github.com/UIPEthernet/UIPEthernet

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024 1

My case in point

Within a templated class' (non-static) method update(), the extern declared obj's tick() should invoke RF24Ethernet.tick() or RF52Ethernet.tick() depending on how the extern ESBEthernetClass was instantiated.

#ifndef RF24_TAP
template<class mesh_t, class network_t, class radio_t>
void ESBEthernetClass<mesh_t, network_t, radio_t>::update()
#else
template<class network_t, class radio_t>
void ESBEthernetClass<network_t, radio_t>::update()
#endif
{
    Ethernet.tick(); // !!! can only resolve to RF24Ethernet.tick(); because
                     // of the static definitions in RF24Ethernet/ethernet_comp.h
}

Using the this pointer isn't an option because ESBEthernetClass::tick() is a static function.

I honestly don't see a way to maintain the current API structure of RF24Ethernet while applying the template approach because of all the static members. I think a custom fork/branch is needed for use with nrf_to_nrf and other radio HW abstractions. See also this SO answer about calling static members from a templated member.

TBH, my first impression is that RF24EthernetClass had a flawed design because the static tick() function calls on externally declared objects (trusting that the user code follows a precise convention). After seeing how UIPEthernet is designed, I'm not sure what a better (template-friendly) design would be.

from rf24network.

TMRh20 avatar TMRh20 commented on September 22, 2024 1

I guess this isn't that big of a deal. RF24Ethernet isn't nearly as popular as the lower layers of the stack, and I don't see many users wanting to run two radios at a time with RF24Ethernet, so we could probably skip the templating of RF24Ethernet and just stick with defines. That is unless you are wanting to re-design it, I think that is a little bit out of the scope of what I can take on right now, and I don't expect you to do everything yourself.
It is fun watching you design code though, its like watching a skilled artist doing his thing, and getting to play around with the results.

from rf24network.

TMRh20 avatar TMRh20 commented on September 22, 2024 1

Yeah, I'm good with all that.

And I think there should be a brief tutorial page somewhere

Probably on the main docs page and in the readme on GitHub, in both RF24Mesh and Network, its hard to get people to read things sometimes. This would be temporary, and eventually we could move it to its own page. This is probably something I can handle, I would just need a bit of time to put it together.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024 1

Please also note that the auto-installers do not do a version check like most other pkg managers.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024 1

I just had a thought: We should keep a separate branch for v1.x in case we need to backport updates. This would be helpful to those that can't/won't migrate to v2, but it would mostly benefit PlatformIO users. I'm not sure if we can retro-release tags for Arduino IDE. I think we can if needed, but we'd be better to do a retro-release (v1.x) before a modern release (v2.x) that way the "latest" release is of v2.x variety.

from rf24network.

TMRh20 avatar TMRh20 commented on September 22, 2024 1

OK, docs are in the template branches if you want to review. I have RF24Ethernet changes for Mesh/Network v2.0 ready to go as well, but I will PR once the other v2.0 changes are in place.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024 1

That's odd. It is supposed to get deployed to PIO registry in the PIO CI when a release is published. I'll have to investigate...

I'm reviewing the cmake installer script, and it was written well enough to not have a significant re-write for the old build system. I'm currently playing around with this idea now. I'm not sure how we could have the old installer forward to the new one though.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024 1

I just checked Platform IO registry and RF24 was still at v1.4.5 for some reason

The logs for that CI run are too old (github removes logs older than 90 days). So, I can't see if it tried deploy to PIO. Given that the CI published the pkg to v1.4.6 release's assets, I think the problem may have been in the CI step's conditional trigger/skip.

Since I updated all the CI to be re-usable workflows (post v1.4.6), I think I still need to add the deploy-release arg to the validate_lib_json step (for all RF24* repos):

  validate_lib_json:
    uses: nRF24/.github/.github/workflows/validate_deploy_platformio.yaml@main
    secrets: inherit
    with:
      deploy-release: ${{ github.event_name == 'release' }}

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024 1

Still having trouble getting RF24Mesh's PIO CI to install the right branch of RF24Network

Run pio pkg install -g --skip-dependencies -l 'https://github.com/nRF24/RF24Network.git#template-attempt2'
Library Manager: [email protected]+sha.05e7f46 is already installed

But that is the wrong SHA for RF24Network's template-attempt2 branch... It should be [email protected]+sha.ecba37cπŸ€·πŸΌβ€β™‚οΈ. Hopefully this will be resolved as the template branches are merged to master (in proper order).

from rf24network.

TMRh20 avatar TMRh20 commented on September 22, 2024 1

I can do the version bump with the releases, you should be able to remove except for RF24Ethernet branch

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024 1

It deployed to PIO! I verified with the CI logs, then I got the email. πŸŽ‰

UPDATE: The network, mesh, and ethernet libs have also been accurately deployed to PIO.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024 1

Ok. Its all merged. We can do a v2.0 release crusade whenever you want.

I also re-released RF24Mesh to make sure installing v1.1.10 or newer doesn't install RF24Network v2.x in PlatformIO. Older versions of RF24Mesh v1 will likely cause an error when used with RF24Network v2.x because there was nothing in RF24Mesh v1.1.9 or previous stopping PIO from using the latest release of RF24Network.

I don't really know if Arduino IDE has version checking for lib deps (see lib specs), but v2 is backward compatible enough for users to migrate. The PIO dependencies were a real pain in the a$$.

from rf24network.

TMRh20 avatar TMRh20 commented on September 22, 2024 1

We can do a v2.0 release crusade whenever you want.

Done!

from rf24network.

Avamander avatar Avamander commented on September 22, 2024

In theory modern C++ provides plenty of functionality to implement different back-ends to provide the same "interface" so to say.

This would also in theory allow clone/semi-clone support.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

In theory modern C++ provides plenty of functionality to implement different back-ends to provide the same "interface" so to say.

Do you have alternate suggestions? I'm looking for something that is the most backward compatible, but I keep landing on polymorphism.

This would also in theory allow clone/semi-clone support.

Absolutely! It would be better to have a separate implementation lib for the RFM7x chips because the setup operations are so much different from the nRF24L01.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

I hesitate to say this because I haven't researched it. An abstracted API (+ pipe count config) might even allow porting the network layers to HW radios that use a different spectrum entirely (ie RFM69).

from rf24network.

Avamander avatar Avamander commented on September 22, 2024

Do you have alternate suggestions? I'm looking for something that is the most backward compatible, but I keep landing on polymorphism.

Though I'm not entirely sure about Arduino IDE and its latest C++ version, but the InfiniTime project is probably a good example how hardware abstraction can be done with newer C++: InfiniTimeOrg/InfiniTime#1387 (comment)

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

Interesting. I think you'll find that different Arduino cores use (or are pinned to) specific versions of compilers. I'm not sure we can guarantee C++20 compatibility in Arduino. There doesn't seem to be a way for an Arduino lib to specify compiler args like -std=c++20.

The selection at build time of the actual interface is done in header files

This has worked well for us in the past for a SPI bus. This doesn't seem geared to building with more than 1 type of HW radio driver. What if you wanted to broadcast 1 network only on channel 90 with 1 radio, and another network only on channel 97 with a different radio?

from rf24network.

Avamander avatar Avamander commented on September 22, 2024

This doesn't seem geared to building with more than 1 type of HW radio driver. What if you wanted to broadcast 1 network only on channel 90 with 1 radio, and another network only on channel 97 with a different radio?

Fully optional really, you can utilize concepts without selecting a specific implementation compile-time. The important part in that case is that you can, without a clusterf**k of #ifdefs. The Basically section really highlights the key components.

There are lower C++ version concepts that could be used as well, like templates.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

No ifdef soup is appealing. I think the deciding factor may be size impact. Since I can't accurately predict how that will go, we may have to prototype the ideas on seperate branches and open dummy PRs to get the report from the Arduino CI. That would also surely highlight differences between toolchains used by different Arduino cores.

from rf24network.

TMRh20 avatar TMRh20 commented on September 22, 2024

So I've been playing around with templates, trying to find a solution to this, and it looks kind of ugly. I'm starting to thing that a bit of ifdef soup might be appealing at this point.
Main issues so far with templates:

  1. Still need to use a bunch of defines to specify the constructor and radio objects.
  2. Needs lots of additional code to implement
  3. Requires changes to any libraries that use the templated library

I don't pretend to fully know what I'm doing here, I'm just learning about templates, but it seems like it creates more problems than its worth.

See the templateSupport branches of RF24Network and RF24Mesh for some incomplete but functional template code with Arduino Pro Mini and NRF52 devices.

from rf24network.

TMRh20 avatar TMRh20 commented on September 22, 2024

I'm still seeing some of the same issues with that approach, things largely still need to be defined, and when switching from RF24 to nrf_to_nrf on the NRF52, I still need to manually add a #define the way its set up right now.

I've got the overrides working for begin() with slightly modified syntax (bool begin(void) override;), but the RF24Network(RF24_API* radio, ...); line is causing problems with the constructor, not being able to convert nrf_to_nrf or RF24 type to RF24_API type. Could be because I'm just learning about overrides now and don't really know how to implement this lol.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

You might have to type cast the nrf_to_nrf obj (or the RF24 obj) when passing it to the c'tor:

RF24Network network((RF24_API*)&radio);

But this might have consequences. Typically, you'd type cast a derivative (AKA "downcast") to access the base class' API. I'm hoping that the pure virtual methods defined in RF24_API (like bool begin() = 0) would forward back to their derived overrides. Again, it's been a while since I played with polymorphism.

No matter what we do here, it would require a breaking change throughout the RF24 stack in terms of instantiating the objects that use the radio HW.

from rf24network.

TMRh20 avatar TMRh20 commented on September 22, 2024

You might have to type cast the nrf_to_nrf obj

Err, that just gives a different variation of the same error.

No matter what we do here, it would require a breaking change throughout the RF24 stack

Not if we just use #defines

I'm starting to think that either way it will be a compromise on how we would ideally like it to work.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

If use defines then it limits the type of radio HW used in a single application, right? I was hoping there was a way to allow various/multiple radio HW types in the same app (eg 1 nRF52 network on channel X and another nRF24 network on channel Y). I don't really have a problem with the ifdef soup, but I was hoping for a more versatile solution.

from rf24network.

TMRh20 avatar TMRh20 commented on September 22, 2024

If use defines then it limits the type of radio HW used in a single application, right?

Yeah, but same with templates as far as I can figure.

One thing I found, even with templates, if you have two objects RF24& radio and nrf_to_nrf& radio, you need to initialize them both in a single constructor. This negates the ability to have separate calls to RF24Network<RF24> network( radio); and RF24Network<nrf_to_nrf> network1(radio1); it would have to be something like RF24Network<nrf_to_nrf> network(radio,radio1);

I've struggled over a day of coding trying to make it work with templates and think I've given up. Learned a lot about templates though lol.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

I don't really follow. You should be able to have separate instantiated objects based on the template param. I just starting to look at what you did in the template-support branch, and I think it could be improved... In what you pushed, you're not really using the template param:

template <>
RF24Network<RF24>::RF24Network(RF24& _radio) : radio(_radio), next_frame(frame_queue)

template <>
RF24Network<nrf_to_nrf>::RF24Network(nrf_to_nrf& _radio) : radio(_radio), next_frame(frame_queue)

Typically it would be more like:

template<typename RF24_like>
RF24Network<RF24_like>::RF24Network(RF24_like& _radio) : radio(_radio), next_frame(frame_queue)

Where the c'tor would be declared in the header file like so:

template<typename RF24_like>
class RF24Network
{
    RF24Network(RF24_like& radio);
    // ...
    RF24_like& radio;
};

And, I think each function definition for the RF24Network class would also need the added template<typename RF24_like> prefixed in the implementation file (RF24Network.cpp)

template<typename RF24_like>
RF24Network<RF24_like>::begin() { /* ... */ }

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

Should also be able to set the default value for the RF24_like param to RF24.

template<typename RF24_like = RF24>
class RF24Network
{
    RF24Network(RF24_like& radio);
    // ...
    RF24_like& radio;
};

But, depending on the C++ std used (specifically C++17), a user could instantiate with a blank set of template args:

RF24 radio(7, 8);
RF24Network<> network(radio);

// with C++17 or later
RF24Network network(radio);

// using non-default template param value
nrf_to_nrf radio1;
RF24Network<nrf_to_nrf> network(radio1);

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

I'm playing with it now (locally). To avoid merge conflicts, I'll post a fair warning if I end up pushing changes to that branch -- or maybe I'll start my own branch.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

Ok, I got it to compile with proper use of template args, but the Adafruit nRF52 Arduino core doesn't use C++17 or newer, so users will have to specify a blank set of template args.

I haven't actually got the HW setup to test this, but the modified helloworld_tx example compiles fine (disregarding warnings about nrf_to_nrf src code).

#include <SPI.h>
#include <RF24.h>
#include <nrf_to_nrf.h>
#include <RF24Network.h>

RF24 radio(7, 8);  // nRF24L01(+) radio attached using Getting Started board
nrf_to_nrf radio1;  // nRF24L01(+) radio attached using Getting Started board

RF24Network<> network(radio);  // Network uses that radio
RF24Network<nrf_to_nrf> network1(radio1);  // Network uses that radio

I also hit a snag in which the compiler wasn't aware of the possible template arg values (even with a default value set), but this SO answer helped satisfy the "undefined reference" errors. Basically, I had to add the following to the end of the RF24Network.cpp file:

// ensure the compiler is aware of the possible datatype for the template class
template class RF24Network<RF24>;
#ifdef ARDUINO_ARCH_NRF52
template class RF24Network<nrf_to_nrf>;
#endif

from rf24network.

TMRh20 avatar TMRh20 commented on September 22, 2024

Nonetheless, it is fixed by adding to the #if defined condition:

Could have sworn I tried that, but must have typed it wrong or something :/

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

I still don't know what to do about Linux. It would seem an ideal candidate for non-RF24L01 HW support, but exposing a specialized template class in boost.python is not a trivial change. So far, my efforts to update the individual wrapper have failed to compile.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

Ok, I had to refactor the pyRF24Network bindings... Should be back to working order (using template class).

I just had a thought that might maintain backward compatibility in user code.

  1. Rename the RF24Network class to be template specific, ie ESB_Network.
  2. Declare a specialization for legacy behavior (in RF24Network.h)
    typedef ESB_Network<RF24> RF24Network;
  3. The nrf_to_nrf lib could optionally use a different specialization
    typedef ESB_Network<nrf_to_nrf> RF52Network;

This compiles fine when used like so in the examples:

#include <SPI.h>
#include <RF24.h>
#include <nrf_to_nrf.h>
#include <RF24Network.h>

RF24 radio(7, 8);  // nRF24L01(+) radio attached
nrf_to_nrf radio1;  // nRF52840 radio

RF24Network network(radio);  // Network that uses radio
RF52Network network1(radio1);  // Network that uses radio1

If this works, it still qualifies as a major change (v2.0.0). The docs might also be a bit off-putting as all doc'd API will be referring to the renamed ESB_Network class.

I haven't fully considered how this will impact RF24Mesh and other higher layers, but I imagine we'd want to do the same template tactic for those layers as well.

from rf24network.

TMRh20 avatar TMRh20 commented on September 22, 2024

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

I'm pushing the renaming changes now (had to re-work the docs a bit). Templates are something that takes practice, and this library isn't simple enough for a learning experience with templates (best to start with a dead simple exercise).

@Avamander We haven't been trying with concepts because the compiler used for the nRF52 cores doesn't have C++20 features enabled. I'm still considering alternatives since I'm pretty sure C++11 std is a common minimum. As suspected, the language standards used varies per core/board, but I found

If we can't rely on a minimum standard, then I think maybe the declared template specializations should adequately limit the supported types of template param values:

RF24Network/RF24Network.cpp

Lines 1290 to 1293 in 0b4cd33

template class RF24Network<RF24>;
#if defined(ARDUINO_ARCH_NRF52) || defined(ARDUINO_ARCH_NRF52840)
template class RF24Network<nrf_to_nrf>;
#endif

While this isn't robust for a multitude of radio HW abstractions, it seems feasible for our current purposes.

from rf24network.

TMRh20 avatar TMRh20 commented on September 22, 2024

So with the RF24Mesh changes, everything is working great for me, but it looks like the constructor for RF24Mesh with nrf_to_nrf is a bit horrific:
ESB_Mesh <ESB_Network<nrf_to_nrf>, nrf_to_nrf> mesh(radio, network); is what I have working...

Is there any way to shorten this up? Probably not but thought I'd ask lol. Still getting used to ESB_Network and ESB_Mesh naming conventions, but I think that is a pretty good choice, but I'd prefer it without the underscore.

When it comes all the way to RF24Ethernet, its going to get ugly.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

The underscore can go. I'm not committed to the names, I just went with what first popped in my head. If feeling playful, we could even use SkyNet & SkyMesh 🀣. Were you thinking of something more like RFx<layer>?

Is there any way to shorten this up? ... When it comes all the way to RF24Ethernet, its going to get ugly.

The typedefs don't just serve as backward compatibility. I have also used the tactic to distinguish between radio type. The following should work as it currently is:

nrf_to_nrf radio;
RF52Network network(radio);
RF52Mesh mesh(radio, network);

As for shortening up the template spec used in the underlying srcs, I'm nor sure. My gut says no, but we might be able to use typedefs instead. I have to think on it. Being familiar with templates, the nested sets of template vars make sense to me.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

While we're still discussing experimental changes, I think RF24Mesh lib could actually inherit from RF24Network instead of wrapping around it.

  • no need for separate network and radio private members in the mesh layer
  • API would be more opaque. Meaning
    • no need to have network.update() exposed when mesh.update() is only needed.
    • network.write() would still be accessible given that the mesh.write() signatures don't resemble network.write() overrides.

This would definitely be a breaking change despite the typedef'd templates:

RF24 radio(7, 8);
RF24Mesh mesh(radio);

The impact of this might be too radical on the other layers though. I'm currently resigned to "don't fix what ain't broke".

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

I dropped the nRF52 commits to RF24Ethernet master and moved them into a separate branch (NRF52-support). You may have to do a git reset though. The following assumes your local clone is on master branch:

cd RF24Ethernet
git reset --hard 0649526c311255ec34ee0e5666df65c5a990e9d8
git pull

I'll be pushing the template idea to RF24Ethernet in a new branch (template-support) shortly.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

Is there a reason that RF24EthernetClass is not named RF24Ethernet? My template approach may obsolete that but break examples which would need to be updated like so:

RF24 radio(7, 8);
RF24Network network(radio);
RF24Ethernet ethernet(radio, network);

// now use `ethernet` obj instead of an obj named `RF24Ethernet`

I can still use the RF24EthernetClass name as a typedef, but I thought I'd ask first.

EDIT: Oh, I see RF24Ethernet is instantiated as an extern obj and called by friend class' definitions. This makes the template approach much more complicated in RF24Ethernet.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

I thought I would give RF24Ethernet a look. Didn't get very far lol.

On the plus side, this issue may have given me a inexcusable reason to familiarize myself with RF24Ethernet. It does seem rather fragile to expect the user code to instantiate an object exactly named RF24Ethernet. Fair warning: I might start pestering you with questions on that repo.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

Thanks, that sheds a lot of light on the design decisions. For now I'll just stick to applying templates to the ethernet layer.


Concerning RF24Ethernet/ethernet_comp.h:

#define Ethernet       RF24Ethernet
#define EthernetClient RF24Client
#define EthernetServer RF24Server
#define EthernetUDP    RF24UDP

I foresee a problem when using these with the RF52Ethernet specialization. We might have to discourage their usage going forward with the template approach. I see they're used in some examples which will only work when using RF24Ethernet specialization. However an app that combines both RF24Ethernet & RF52Ethernet will have to use more explicit references (not the above aliasing definitions).

from rf24network.

TMRh20 avatar TMRh20 commented on September 22, 2024

Ok, I think I have a good basic handle on how templates work now, thanks to all your examples with code I already understood. Its so much easier to follow a working example than random examples on the internet. I've tested what I can and think we can merge this stuff as soon as you think its ready @2bndy5 .

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

I'm ready to move on this also. I'm only cautious because this would mean releasing a v2.0.0 tag. And I think there should be a brief tutorial page somewhere (probably in RF24Network) about migrating to v2. I say brief because it shouldn't be a major impediment, but there are caveats:

  • Any network layer that uses v2 needs to have RF24Network/RF24Mesh dependencies of v2 or newer. RF24 v1.x is an exception here.
  • Any third party libs that extend the network/mesh layer may also need to be updated to incorporate the new templated class prototypes.
    template<class radio_t>
    class ESBNetwork;
    
    template<class network_t, class radio_t>
    class ESBMesh;
    The entire template tactic might also need be applied to third party libs, which as we've seen in RF24Ethernet may be a non-starter.

I haven't played around with this much, but I think that third party libs can use the backward-compatible typedef in their template spec if they use typename instead of class as the template arg's datatype:

template<typename network_t, typename mesh_t>
class ESBGateway

And also inform the compiler what types they intend to support

template class ESBGateway<RF24Network, RF24Mesh>;

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

The .github/docs uses jekyl to render MD files as a webpage. I'm not sure how handy it is for multiple pages given the current setup (which scrambled together rather clumsily).

from rf24network.

TMRh20 avatar TMRh20 commented on September 22, 2024

Great idea, if you would take care of the releases since you did most of the work I think we are ready for 1.x releases all around.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

FYI, the cmake-based auto-installer has the ability to select a branch for each layer. But this functionality was never exposed to the CLI; it was actually used for testing/developing. I'm guessing it would be a good idea to ask the user for a branch name (defaulting to master) before it installs the lib. Instead of a branch name, I think it could also use a git tag, but I think the installer has to do a git fetch --all to checkout a tagged commit.

The cmake-based installer will only work for versions that have CMake support. If we try to fix that the install script would have to become a lot longer, so it could fallback to the old build/install system (if no root CMakeLists.txt file is present).

from rf24network.

TMRh20 avatar TMRh20 commented on September 22, 2024

Lets not get too complicated here, I don't think we need to worry too much about older versions, as long as the newer versions have CMake support, we should be OK.

Also I just checked Platform IO registry and RF24 was still at v1.4.5 for some reason so I pushed the current code as 1.4.6. The other libs are up-to-date. Just something we need to check on after our next releases.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

The installer script mods are at the .github repo's install-versions branch. It doesn't seem to work with WSL using RF24-v1.3.11 and RF24Network-v1.0.15 (linker ignores arm-specific .so file for building RF24Network), but I pushed the branch to test it on an actual Linux (32bit) device.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

CI updated org-wide. 🀞🏼 I also rebased the relevant template-* branches to avoid merge conflicts.

from rf24network.

TMRh20 avatar TMRh20 commented on September 22, 2024

So are we ready to do this thing? You want me to do the 1.x releases?

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

Sorry I've been dragging my feet no real reason here. Yeah, we're ready for release crusade.

I didn't see anything noteable in the in the net work layer histories since our last crusade. RF24 lib got some significant updates in the PicoSDK support, but that has been in master for a while and I suspect that people are using master instead of a tagged commit with the PicoSDK because our PicoSDK instructions don't explicitly say to use tags.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

@TMRh20 If you want to do the releases (should be easy with that auto-generate summary button), then I'll open the PRs and start the v1.x branches.

I almost forgot about the version bump in the lib.properties/json files.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

Is it ok to remove the initial NRF52Support and templateSupport branches that you started?

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

RF24Gateway has only had CI changes since last release. It might be a good idea to make sure the CI deploys to PIO for RF24 v1.4.7 before continuing to publish releases for other layers because they'll all be using the same PIO validate/deploy workflow.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

v1.x branches created

I created a branch rule for each of the new v1.x branches on the network, mesh, and gateway layer. This way the git history should be respected the same as it is for the master branches.

v2.0 PRs open

I left some open as drafts to indicate that they aren't ready to merge unless the dependency layer has been merged. This is done to remind me to clean up the CI changes I used for testing.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

I know this isn't a goal, but if there are any breaking API changes you want make (like renaming functions with underscores to not have underscores), then this is the chance to do so. I mention this because semantically, it makes sense to remove deprecated functions (or rename functions) in a major version bump.

On the topic of deprecated functions:
RF24Network::begin() has a deprecated overloaded function that techincally would be removed on a major version bump. However, I doubt there would be enough noticable change in compile size to deem it ok to remove.

from rf24network.

2bndy5 avatar 2bndy5 commented on September 22, 2024

So sorry. I'm going to have to step away again... life and whatnot. The PRs look good at a glance, I found some areas where the docstring mods should more explicitly reference the nrf_to_nrf lib. And the usual CI reverts before merging...

from rf24network.

TMRh20 avatar TMRh20 commented on September 22, 2024

No rush, the 1.x release is in successfully, so whenever you get a chance for 2.0 its all good.

from rf24network.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    πŸ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. πŸ“ŠπŸ“ˆπŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❀️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.