Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mseed crate doesn't support to create MSRecord by caller #14

Open
BIBIN-EUGINE opened this issue Sep 1, 2023 · 2 comments
Open

mseed crate doesn't support to create MSRecord by caller #14

BIBIN-EUGINE opened this issue Sep 1, 2023 · 2 comments

Comments

@BIBIN-EUGINE
Copy link
Contributor

BIBIN-EUGINE commented Sep 1, 2023

Hi @damb , I have come across with using mseed::pack_header2(msr: &MSRecord, buf: &mut [u8]) , but I am not sure how to create a MSRecord as it's a pointer to MS3Record and doesn't provided any API to initialize it.

It would be nice if there is a example for how to use this API for the caller.

Thanks

@damb
Copy link
Owner

damb commented Sep 2, 2023

I guess this is somehow related to #13.

There is a reason why there isn't an example, yet. At the time being, this is simply not possible. However, while designing and implementing the API I knew already about these kind of issues.

Currently, MSRecord can be instantiated only from a miniSEED record buffer. With this approach a user of MSRecord can be sure that it has always a valid and fully initialized instance of MSRecord. No invariants are required. On the other hand, we loose some flexibility.

Now to your problem. Basically, I see two approaches to solve this issue.

  1. Trade simplicity against flexibility. Allow to partially initialize MSRecord which requires several invariants and a complete rework of the MSRecord API.
  2. Providing overloads for both pack_header2() and pack_header3() which allow packing headers from mseed::PackInfo.

For the sake of simplicity, I'd prefer to implement the latter approach. However, there may be reasons I'm still not aware of and in the long term it might be better to have more flexibility.

What do you think?

@damb damb changed the title mseed crate doesn't support to create MSrecord by caller mseed crate doesn't support to create MSRecord by caller Sep 3, 2023
@BIBIN-EUGINE
Copy link
Contributor Author

I guess this is somehow related to #13.

There is a reason why there isn't an example, yet. At the time being, this is simply not possible. However, while designing and implementing the API I knew already about these kind of issues.

Currently, MSRecord can be instantiated only from a miniSEED record buffer. With this approach a user of MSRecord can be sure that it has always a valid and fully initialized instance of MSRecord. No invariants are required. On the other hand, we loose some flexibility.

Now to your problem. Basically, I see two approaches to solve this issue.

  1. Trade simplicity against flexibility. Allow to partially initialize MSRecord which requires several invariants and a complete rework of the MSRecord API.
  2. Providing overloads for both pack_header2() and pack_header3() which allow packing headers from mseed::PackInfo.

For the sake of simplicity, I'd prefer to implement the latter approach. However, there may be reasons I'm still not aware of and in the long term it might be better to have more flexibility.

What do you think?

I agree, overloading it with mseed::PackInfo make it easy to implement for now. However, it would be nice to refactor the MSRecord API to bring more flexbiltiy into it

@BIBIN-EUGINE BIBIN-EUGINE reopened this Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants