FactbirdHQ / atat

no_std crate for parsing AT commands
Apache License 2.0
109 stars 29 forks source link

make Client public to allow tighter control over how it is created #187

Closed dragonnn closed 9 months ago

dragonnn commented 9 months ago

I can not think of a reason why Client pub new couldn't be public. With this change an user can full create a ResponseChannel, UrcChannel, Ingress and the Client by them self. This allows doing something like that:

    static RESPONSE_CHANNEL: ResponseChannel<INGRESS_BUF_SIZE> = ResponseChannel::new();
    static URC_CHANNEL: UrcChannel<Urc, URC_CAPACITY, URC_SUBSCRIBERS> = UrcChannel::new();
    static DIGESTER: DefaultDigester<Urc> = DefaultDigester::<Urc>::default();

    let ingress = Box::new(Ingress::new(
        DefaultDigester::<Urc>::default(),
        RESPONSE_CHANNEL.publisher().unwrap(),
        URC_CHANNEL.publisher(),
    ));

    let client = asynch::Client::new(
        tx,
        &RESPONSE_CHANNEL,
        atat::Config::default().cmd_cooldown(core::time::Duration::from_millis(5).try_into().unwrap()),
    );

The advantage in that is we can create Ingress directly in a Box to avoid it coping from stack to heap when putting in a Box (as the compiler should hopeful optimize that out) and then when moving ingress around we can avoid doing more copy on move of ingress with might be not optimized out like when putting it into a task. A better solution for the future would be to allow putting a static buffer from outside Ingress but that will be a bigger rework and I think having Ingress::new public still isn't a bad idea, all other parts used by Buffers are already public anyway.