lib: Add support to navigator v2 in pi4 and navigator v1 to pi5

[patrickelectric]

Signed-off-by: Patrick José Pereira [email protected]

[RaulTrombin]

@patrickelectric pls check inbox, I sent a quick sketch using builder with enum options:

    let mut nav = Navigator::create()
        .with_board_version(BoardVersion::V2)
        .with_pi_version(PiVersion::Pi5)
        .with_rgb_led_strip_size(8)
        .build();

[patrickelectric]

@patrickelectric pls check inbox, I sent a quick sketch using builder with enum options:

    let mut nav = Navigator::create()
        .with_board_version(BoardVersion::V2)
        .with_pi_version(PiVersion::Pi5)
        .with_rgb_led_strip_size(8)
        .build();

This adds a syntactic sugar that just adds complexity in my opinion. Of course the code will "look better". But the same logic will exist in the end in the backend and the enums will also add complexity for the c++ and python bindings, the original implementation is simpler and easy to maintain for c++ and python, so I would shot for simplicity.

     let mut nav = Navigator::create()
         .with_board_version(BoardVersion::V2)
         .with_pi_version(PiVersion::Pi5)
         .with_rgb_led_strip_size(8)
         .build();
//vs
     let mut nav = Navigator::create()
         .with_navigator_v2_pi5()
         .with_rgb_led_strip_size(8)
         .build();

[RaulTrombin]

@patrickelectric pls check inbox, I sent a quick sketch using builder with enum options:

    let mut nav = Navigator::create()
        .with_board_version(BoardVersion::V2)
        .with_pi_version(PiVersion::Pi5)
        .with_rgb_led_strip_size(8)
        .build();

This adds a syntactic sugar that just adds complexity in my opinion. Of course the code will "look better". But the same logic will exist in the end in the backend and the enums will also add complexity for the c++ and python bindings, the original implementation is simpler and easy to maintain for c++ and python, so I would shot for simplicity.

     let mut nav = Navigator::create()
         .with_board_version(BoardVersion::V2)
         .with_pi_version(PiVersion::Pi5)
         .with_rgb_led_strip_size(8)
         .build();
//vs
     let mut nav = Navigator::create()
         .with_navigator_v2_pi5()
         .with_rgb_led_strip_size(8)
         .build();

hmmm can't see how it'll increase complexity for next libraries, they'll even have the same advantages following this builder pattern.
If you check the sketch, it's merging the repeated content from 4 functions into a single common one with their specific changes.
Still think It would be easier for the final user/consumer of library and future use cases/platforms

Just in case @joaoantoniocardoso wanna check the proposal:
https://github.com/RaulTrombin/navigator-rs/tree/pi5_helps_patrick

[patrickelectric]

@patrickelectric pls check inbox, I sent a quick sketch using builder with enum options:

    let mut nav = Navigator::create()
        .with_board_version(BoardVersion::V2)
        .with_pi_version(PiVersion::Pi5)
        .with_rgb_led_strip_size(8)
        .build();

This adds a syntactic sugar that just adds complexity in my opinion. Of course the code will "look better". But the same logic will exist in the end in the backend and the enums will also add complexity for the c++ and python bindings, the original implementation is simpler and easy to maintain for c++ and python, so I would shot for simplicity.

     let mut nav = Navigator::create()
         .with_board_version(BoardVersion::V2)
         .with_pi_version(PiVersion::Pi5)
         .with_rgb_led_strip_size(8)
         .build();
//vs
     let mut nav = Navigator::create()
         .with_navigator_v2_pi5()
         .with_rgb_led_strip_size(8)
         .build();

hmmm can't see how it'll increase complexity for next libraries, they'll even have the same advantages following this builder pattern. If you check the sketch, it's merging the repeated content from 4 functions into a single common one with their specific changes. Still think It would be easier for the final user/consumer of library and future use cases/platforms

hmmm can't see how it'll increase complexity for next libraries,

I'm trying to avoid enums in the rework to avoid complexity in the C++ and python bindings.

If you check the sketch, it's merging the repeated content from 4 functions into a single common one with their specific changes. Still think It would be easier for the final user/consumer of library and future use cases/platforms

I agree, but I believe that I'm inclined more to a point where the boards configuration can be isolated, adding the duplicated code, makes the logic simpler to review and debug. Merging the boards configuration with functions that remove the duplicated code or move the syntax to be prettier just will add unnecessary complexity IMO, increasing maintenance and work just for the sake of "pretty".

The code is dead simple and ready to use.

@joaoantoniocardoso two cents ?

[joaoantoniocardoso]

I think while @patrickelectric's proposal is enough, @RaulTrombin's proposal is improving for both the user and the developer sides, at least if we look at the current state.

Then, this made me think for a while:

I'm trying to avoid enums in the rework to avoid complexity in the C++ and python bindings.

Right, this is a very strong point, and I have the same concern, as rust enums are known to be a distinct part of the language, possibly not widely compatible with other languages.

Simply thinking of 1:1 translations, it's probably fine if we constrain it as:

Rust:

#[repr(C)]
#[derive(Debug, Clone, Copy)]
pub enum BoardVersion {
    V1 = 1,
    V2,
}

then, as it's not a data-carrying enum, it can easily be translated into the other two languages:

C/C++:

typedef enum {
    BoardVersion_V1 = 1,
    BoardVersion_V2,
} BoardVersion;

Python:

from enum import IntEnum

class BoardVersion(IntEnum):
    V1 = 1
    V2 = 2

---

now, if a 1:1 translation is not enough for us, or we plan to use data-carrying, then I'd say it's probably not worth investing in that just to have that enhancement on top of what already works.

Here is the material we should read before going forward.

[RaulTrombin]

We also already deal with enums bindings on navigator-lib, is the case for PWM Channels and ADC, vagely remember if directly or on cpy-binder, would be nice to define some standard.

The proposal was just to split common functions and follow-up the builder architecure with rust's typed echosystem.
I dont see the enums as blockers as we redefine the functions on navigator-lib, the funcions there work like adapters of rust's ones, accepting any kind of specific signatures. ( is the current state were we have the project in)

But I agree that the actual proposal already work.

[patrickelectric]

@RaulTrombin @joaoantoniocardoso I added the build pattern but with the original implementation in place. I think this comes with the best of both.

[joaoantoniocardoso]

I'm fine with it