Additional optional sample rates in SoapySDRPlay driver

@SDRplay
Andy, I am creating this new issue in the SoapySDRPlay driver to ask your opinion about the following question.
Franco Spinelli IW2DHW (@frspin) and I have been working for some time to get John Melton's G0ORX linphsdr program to  work with the SDRplay RSPs  using the SoapySDRPlay driver (see my pull request to Johns' repository here: https://github.com/g0orx/linhpsdr/pull/84)
John's program uses a DSP library called WDSP (https://github.com/g0orx/wdsp) by Warren Pratt NR0V, that has been specifically designed for the HPSDR class of SDRs; these SDRs work with sample rates in the 48kHz 'family', i.e. 48kHz, 96kHz, 192kHz, and 384kHz.
Based on the discussions with you and Vincent a while ago (and based on what SDRuno presents to the user), I was under the impression that the RSPs would work best with output sample rates of 2MHz (and 1M, 500k, 250k, 125k, 62.5k using decimation) with an IF of 1620kHz, and with sample rates > 2MHz with a zero IF.
What we would be interested instead is to be able to use them with a sample rate of say 1536kHz and a zero IF (which with decimation would yield all the sample rates above, since 1536/4=384, 1536/8=192, and so on).
Franco (@frspin) modified his own copy of the SoapySDRPlay driver, ran same tests, and it looks like this configuration would work, but, not being familiar at all with the internals of the RSPs and the libsdrplay_api library, I wanted to ask your counsel before moving forward.
If you think these sample rate values are acceptable and do not cause other problems, I thought they could be added to the SoapySDRPlay driver, either in the main code, or perhaps using a 'cmake' compilation flag (like we used to to have for the RF gain thing a while ago), or perhaps in a different repo branch (which might make things a little more complicated since the two branches would have to be kept in sync after that). Another option would be for us to create a fork somewhere with these additional optional values of the sample rate, but I am afraid it could cause unnecessary confusion among the customers.
Thanks,
Franco

Hi Franco,
Franco Spinelli logged a support ticket with this issue and you can get to lower arbitrary sample rates, but only in Zero-IF mode because you need a starting sample rate of between 2 and 10 MHz
So 384 kHz = 3.072 MHz with a decimation of 8 and 768 kHz would be 3.072 MHz with a decimation of 4 etc.
Obviously the only issue to be aware of with Zero-IF is the DC offset correction algorithm, so you would need an offset between the LO and the VFO for signals such as AM, although for FM this wouldn't be a problem.
Hope that helps,
Best regards,
Andy

@SDRplay
Thanks Andy - that helps a lot!
The question that I'd like to discuss next is how to best implement this set (or range) of sample rates in the SoapySDRPlay driver, since I think most of the customers use it with the CubicSDR application, and I would like to avoid (or limit) changes to the way they currently see the RSP devices and their sample rates.
There are two main places in the SoapySDRPlay driver where the client application deals with sample rates (just to make sure we are all on the same page, by 'sample rates' in this discussion I am always referring to the 'output sample rates', i.e. the sample rates as seen by the client, as per our discussion with Vincent a while ago):

listSampleRates(): it currently returns a list of allowed sample rates (62.5kHz, 125KHz, etc, all the way to 10MHz)
setSampleRate(): this one calls getInputSampleRateAndDecimation(). I just looked at the code of getInput...(), and currently it basically has two big branches:

if the output sample rate is <= 2MHz, it sets an input sample rate of 6MHz (or 8MHz), and an IF of 1620kHz (or 2048kHz); the decimation is set to 32, 16, 8, etc based on the selected output sample rate of 62.5kHz, 125kHz, 250kHz, etc
if the output sample rate is > 2MHz, the input sample rate = output sample rate, selects Zero_IF, and of course sets the decimation factor to 1

In light of the current behavior, I was considering a few options (@frspin feel free to add your suggestions too):

for listSampleRates():

listSampleRates() could stay unchanged, presenting the same list of sample rates as we do know, basically ignoring the fact that other sample rates are available
we could decide that we want to present only a discrete set of additional sample rates that "make sense", like 96kHz, 192kHz, 384kHz, and 768kHz, and just add those 4 or 5 additional sample rates to the list returned by listSampleRates()

for setSampleRate() (i.e. getInputSampleRateAndDecimation()):

if we choose to only implement an additional set of well defined sample rates (96kHz, 192kHz, etc), then I could add another 'if' statement based on those specific values; if the output sample rate selected by the application is one of those extra values, I would return an input sample rate of 3.072MHz, set Zero_IF, and a decimation factor that depends on the output sample rate, as discussed above
if we instead we decide we want to allow for an arbitrary range of low sample rates, then things become a little tricky; for instance, imagine a request to set the output sample rate at 500kHz: it could be done with the current settings (fsHz=6M, dec=4, IF=1620), but it could also done using a Zero_IF this way: fsHz=4M, dec=8, IF=0 (similarly to what we would be doing for say 384kHz). It also opens another set of problems: if the user chooses an output sample rate of say 499kHz (or 495kHz), should we round it to 500kHz, and use the 1620kHz IF, or should we leave it unchanged and select a fsHz=3.992MHz (or 3.96MHz), Zero_IF, and dec=8?

Franco

Hi Franco,
I think there will always need to be a consideration between what we could do vs what the SoapySDR framework was designed to support. The classic case being the gain control issue.
My understanding is that SoapySDR was really designed to support a range of predefined sample rates, hence why there is a listSampleRates function. This has presumably been modelled on the ExtIO format where the exact same thing is done.
I would personally stick with the predefined list of sample rates which is easier to control and more in keeping with the SoapySDR framework. It is true to say that 2 MHz, 1 MHz, 500 kHz should all be delivered using Low-IF, beyond that the others you mentioned will all have to be Zero-IF. You really want to keep the sample rate as low as possible for 2 reasons...

CPU load - yes you could use 6.144 MHz instead of 3.072 MHz but then the decimation filter needs to be twice as high for the same final sample rate, so not only is there an extra load on dealing with the higher input sample rate, there is also more software processing to deliver the final sample rate.

ADC bits - With higher sample rates, comes lower number of bits - below 6.048 MHz, the number of bits is 14, above 6.048 MHz the number of bits goes to 12, then above 8.064 MHz it's 10 and for > 9.216 MHz it's 8 bit. This was designed to make the data all fit within the USB 2.0 bandwidth limit.
So 3.072 MHz is 14 bits, but 6.144 MHz is 12 bits just to decimate down to the same final sample rate. Yes, I know that effectively every divide by 4 in the decimation filter gets you 1 bit back, but decimating 6.144 MHz down to 384 kHz only gets you back to 14 bits, which is where 3.072 MHz starts at. I don't want to get into the whole number of bits debate because at HF other factors dominate anyway, but that's for a different discussion. But you see what I mean in general.

As an algorithm, you can start with the final sample rate and keep multiplying by 2 until you get equal to or above 2 MHz and then that becomes your input sample rate.
Does that stack up with what you want to do?
Best regards,
Andy

Hi @SDRplay, hi @fventuri !
Just my 2 cents here on CubicSDR behaviour:

SoapySDR::listSampleRates retrives a list of predefined samples rates, that are displayed in ascending order in CubicSDR,
Cubic also add a Manual entry menu where the user can input an arbitrary sample rate between the min and max of the ones provided by the SoapySDR::listSampleRates.
In all cases, the sample rate is eventually set through SoapySDR::setSampleRate

So basically a user can input any sample rate he wants.
Another small point, while I agree with @SDRplay for the others:

CPU load - yes you could use 6.144 MHz instead of 3.072 MHz but then the decimation filter needs to be twice as high for the same final sample rate, so not only is there an extra load on dealing with the higher input sample rate, there is also more software processing to deliver the final sample rate.

IMHO, that part is quite small compared to the other DSP processing done later by any SDR application, we should not limit the bandwith if it is beneficial.
About the ADC bit part, I agree. Personnaly I stick to 8MHz all the time with my RSP2 because if gives the best apparent results: In particular, the other lower sample rates give worse images an dirtier signals, so I keep using 8MHz whatever the RF band I'm looking at.

And therein lays the problem. What I see as a developer a lot of the time, is that each of us has an experience which is different to the other and sometimes that can "bleed" into the development. In truth the API can deal with any scenario you throw at it, the reason I prefer to use low sample rates is because I'm typically using SBCs where every bit of processing power is precious. Whilst Vincent uses 8 MHz sample rate because that suits his setup.
Neither of those scenarios have really anything to do with the hardware or the API, it's either the host we choose to use or the environment (atmosphere, antenna, cable, interference, etc.) that mostly dominate.
I don't really have a preference as to how the sample rate is delivered, if the SoapySDR/CubicSDR environment can cope for arbitary sample rates, then the API can deal with that if you choose to implement it.
Just as a word of caution, when we started SDRuno, we gave the user the ability to tweak everything the API could do. That was a massive mistake, we have learned that people really want less options, but more optimisation for those options. What's really more important here than the sample rate, is what IF bandwidth you select. You have to avoid aliasing and make sure that the IF bandwidth is appropriate for the sample rate selected (whatever it ends up being).
Best regards,
Andy

@SDRplay @vsonnier
Thanks for the very useful and insightful comments, Andy and Vincent!
I think Andy's last comment about trying to keep things simple and give the user fewer but well tuned (and well tested) choices is very important.
Another principle I tend to stick with is the principle of 'least surprise', which basically means that if something used to work in given way (for instance using a LIF for sample rates of 65.5kHz, 125kHz, etc), do not change it unless it is absolutely necessary and for very very good reasons, because if you do, it is going to confuse the user, possibly have other unforeseen consequences, and most importantly tarnish the feeling of stability in the application/library (i.e. "they keep changing things  around"; how many times we hear that!)
Also I suspect that most (if not all) of the users use one of the predefined settings for the sample rate, i.e. the 'manual' setting in CubicSDR mentioned by Vincent is seldom used (if at all). Please correct me if I am wrong; I'd like to hear what are the reasons  for selecting an output sample rate of say 100kHz, or 495kHz.
If these premises are correct (especially the one about 'manual' sample rates), I think the best approach to add support for 'linhpsdr' (and possibly other applications of the HPSDR family in the future) would be to just add the four sample rates: 96kHz, 192kHz, 384kHz, and 768kHz, as suggested by @frspin, and they would be implemented with a fsHz=3.072kHz, zero IF, and appropriate decimation factor.
I also think that we should deal with values of requested sample rates that are <=2Mhz and not in the list of selectable values, by flagging them as invalid, return an error message to the user, and do not attempt to change the current sample rate. I am saying this because, after looking at the current code in the SoapySDRPlay driver, @frspin made me realize that if a user enters a manual sample rate  of say 350kHz, the code would return an input sample rate of 6Mhz, an IF=1620kHz, and a decimation factor of 1 (i.e. the values for an output sample rate of 2MHz), which is wrong.
If we all agree on this approach, I can work on making these changes (in a new branch) over the weekend, we can run some tests with the various applications and use cases, and then make a final decision.
Franco

Sorry Franco,
You're right, I should have said that 2 MHz, 1 MHz, 500 kHz, 250 kHz, 125 kHz, 62.5 kHz should all be done with Low-IF, the rest will need to be Zero-IF
I've never used the arbitrary sample rate feature in CubicSDR - I didn't even know it existed or that the SoapySDR framework supported it!
Best regards,
Andy

Well then, let me try to come at it from a different perspective. The focus on CubicSDR is not a good practise to start with, there's other applications out there where sample rates may have a completely different meaning: I'm talking about OpenWebRX, where the sample rate choice is mostly based on what band you're on and how much of the spectrum you'd like to share.

Limiting the list of sample rates to a predefined list seems a bit arbitrary to me. The fact that the very commonly used sample rates 2, 2.048 and 2.4 MS/s are currently not listed by SoapySDRUtil --probe (which I presume calls listSampleRates() internally) has already led to confision, ref: https://groups.io/g/openwebrx/topic/76989259
If the hardware is capable, how about reporting sample rate ranges instead of pre-determined values?
If you condense this down to a list, please make sure that all entries in the list are either supported by all the hardware, or filter the list accordingly before presenting it to the user. I'm stating this mostly because of this very similar issue that has popped up on my end where the RFGR setting is incorrectly reported as a range [0..9] for an RSP1.
Whatever you do, please also make sure that the values passed to setSampleRate() is checked and an error is returned if the sample rate is unsupported. I have experienced first-hand that this is not the case for Master/Slave mode, where I set the sample rate to 2.048 MS/s in error. The problem is that this is close enough to 2 MS/s (which is quietly chosen in that case) that it took me hours to notice.

@jketterl
Thanks for the feedback Jakob!
As a developer it is very useful for me to see other people's perspectives, understand how the driver is used, and how to make it better and more useful.
The way I see it is that the SoapySDRPlay driver (and the SoapySDR framework in general) is trying to abstract the internal complexity of the low level hardware, and provide a sort of unified 'experience' and API to the various applications that use it; in my mind, I see a stack more or less like this: a low level library that talks directly to the hardware (like libsdrplay_api or librtlsdr) at the bottom; an 'intermediate' level API (like SoapySDR, or a GnuRadio OOT module) that provides a well defined and unified API (and because of being unified, many times it is a simplified abstraction of the underlying low level API), and finally a client application on top, like OpenWebRX, or CubicSDR (or for instance gqrx in the case of GnuRadio).
The important thing to notice here is that the intermediate level layer has sometime to make some trade-offs (or choices) between presenting a unified and consistent API to the client and the 'quirks' of the underlying library/hardware; case in point here is what to do with sample rates below or equal to 2MHz for the SDRplay RSPs: a request for say a sample rate of 500kHz could be implemented in two ways, with the LIF approach, or with the ZIF approach, but the setSampleRate() call does not have a way to specify that (and rightly so, because this is a high level API that shouldn't "bothered" with this kind of details), so the intermediate level driver has to manage this case in some way. This problem can also get even trickier when you try to implement a continuous range of sample rates, where say a request for a sample rate of 499.5kHz would be implemented one way (using ZIF), and a request for a sample rate of 500kHz would be implemented in a different way (using LIF) . A user playing around with the sample rates settings might change the sample rate from 500kHz to say 499.5kHz, notice that things are very different, and think your application has a bug because of that.
Also we need to consider is that another useful purpose of this intermediate level is to simplify things to the client application (and therefore to the end user, since many client applications like CubicSDR present that list of sample rates directly to the user) in a way that the user does not have to "think too much" about what to select (like Andy pointed out in his comments about the evolution of the UX in SDRuno); presenting a well defined set of sample rate values all implemented in a well defined way could be more useful than returning a range of [min,max] and hoping for the best.
Finally I agree 100% regarding printing an error message and returning an error for invalid sample rates; the problem in this case is that the Soapy driver API defines setSampleRate() as a function returning a void (see: https://github.com/pothosware/SoapySDR/blob/926c86d98b3a1d0b948ef6252982ee55230f1f0c/include/SoapySDR/Device.hpp#L873), so the developer is left to scratch their head on how to handle errors inside setSampleRate(): throw an exception? call abort()? (just kidding), perhaps there is a better way?
This all said, I am not at all against adding other possible sample rates below 2MHz, if we know that they are useful and used in one of the many applications that use this driver; I think we can just come up with a list, explicitly state how to uniquely implement them (LIF for 62.5, 125, 250, etc; ZIF for the others), and I'll be more than happy to change the code to do that (of course the list of sample rates is not cast in stone; it could be modified in the future as we see fit).
Franco

The best approach, from my experience, is to just provide both, i.e. an API that provides reasonable defaults so you can get usable results in a very quick fashion, but then also expose the advanced settings for those that know what they're doing.
Lower sample rates do make sense for OpenWebRX, especially on shortwave, where there is some very narrow HAM bands (the narrowest is 30m I believe, being only 50kHz wide). Some users have experienced aliasing issues with the lower sample rates, though (https://groups.io/g/openwebrx/topic/strange_behavior_with_rsp1/78060989). I haven't had much time to experiment with it, and maybe there is a way around this, but this suggests that there's a lower limit somewhere.
The high sample rates, on the other hand, are useful for spanning multiple bands at the same time. The main use case for this is spotting / skimming applications, or maybe from 70cm upwards. Both require some CPU grunt, though, of course.
As for the error handling: I did not know that the C++ API specifies a void return type, the correspondig C API that I've been using returns an int. That conversion seems to be done by catching exceptions, so I suppose that's probably the best way to go.
See:
https://github.com/pothosware/SoapySDR/blob/master/lib/DeviceC.cpp#L562
https://github.com/pothosware/SoapySDR/blob/master/lib/ErrorHelpers.hpp#L13

@jketterl
Thanks Jakob for your feedback.
On the high sample rates (i.e. above 2MHz) the current approach of fsHz=<sample rate>, Zero IF, and decimation factor=1 should work just fine, and should allow (at least in the call of the setSampleRate() function) to set any value you want.
It's with the lower sample rates (<= 2MHz) that things get tricky.
In the specific example you mention of the 30m band, with a 50kHz bandwidth, would your users be OK with an I/Q (quadrature) sample rate of 62.5kHz, for instance, instead of exactly 50kHz?
The SDRplay RSPs also have a set of bandwidth filters, but the narrowest one is 200kHz (if I remember correctly), which means that when you go down to those very low sample rates, there is so much that the RSP hardware can do for you, and the user may need additional external filtering to avoid aliases.
Another factor to consider, which I didn't mention in my previous comment, is that the RSPduo hardware in dual tuner or master/slave configuration has to work in LIF mode, i.e for that specific hardware in that specific configuration, the only sample rates  possible are 62.5kHz, 125kHz, 250kHz, 500kHz, 1MHz, and 2MHz, and that's it.
I plan to make the changes for the additional sample rates over the weekend, and put them in a different branch; after that,  we can run some tests with different clients, different hardware, and different configurations, see how it goes, and see how this driver can be improved.
Also if you know of other applications using this driver, I think it would be a good time to invite their developers to join this discussion, so that we can reach a large consensus on this subject.
Franco

Hello all, I've added a commit to CubicSDR : https://github.com/cjcliffe/CubicSDR/commit/7b1956f7cd48e9adb520d862814573dd3130e59f
This commit attempts to better manage the case when the user request a setSampleRate incompatible with the hardware.

Catching exception on setSampleRate : this is really bad, please do not do this, though.
Irrespective of 1., always re-read the actual sample rate by getSampleRate and register it as the effective user setting.

Throwing execptions is bad because nothing in this API suggests that a call may fail in such brutal way. We cannot reasonnably force SoapySDR applications to wrap every API call into try-catch blocks and manage it that way.
IMHO, this API must be exception-free.
On the other hand, for each set there is always a get where a client application can read the current effective settings, as an option.
All the discrete sample rates of the RSP that cannot be applied exactly should indeed be implemented that way:

The user application request a particular sample rate by setSampleRate
The driver make a best effort to apply the requested sample rate (ex, > 2MHz). If it cannot, use the closest discrete, valid, samplerate supported and apply it instead.
The user application can later use the getSampleRate call to retrieve the really applied sample rate and try to manage the difference at the application level.

Throwing execptions is bad because nothing in this API suggests that a call may fail in such brutal way. We cannot reasonnably force SoapySDR applications to wrap every API call into try-catch blocks and manage it that way.
IMHO, this API must be exception-free.

How can you expect to just call a method and not perform any kind of error handling?
Since the API does not allow passing a result as a return value, exceptions are the only thing that I can think of on how to handle any kind of error. This is done in SoapySDR itself, too, see: here or here.
I'm not aware of any guiding documentation for implementing device modules, if there's any it would certainly be helpful. So far I'm only reading the code, and there's multiple indications that exceptions will be and should be thrown. That's at the level of SoapySDR, so deviating here is a bad idea.
As for your suggestion about guessing the sample rate: It would be a lot of work to implement it this way for me, and would still be confusing. As stated before, a corresponding error would work better for my application.

Since the API does not allow passing a result as a return value, exceptions are the only thing that I can think of on how to handle any kind of error.
That's at the level of SoapySDR, so deviating here is a bad idea.

This is not in the contract or in nowhere the general documentation. This is even worse than a return value, because we don't know the type of exceptions supposed to be thrown.  And no, std::runtime_error is not SoapySDR specific and convey no useful information.

This is done in SoapySDR itself, too, see: here or here.

Those examples are not that good. They cover device initialization and deinit, which may not fail except in rare circumstances.
On the other hand, thowing exceptions all over the place while the device has actually been initialized properly, while the application has previously configured the driver according to its advised capabilities (valid sample rates in particular), is certainly not an expected runtime beahaviour, IMHO.

As for your suggestion about guessing the sample rate: It would be a lot of work to implement it this way for me,

This is what the RSP driver does already choosing internal settings according to the requested setSampleRate.  (decimation, Zero-If vs. Low-IF...etc.) and this is what make this driver complicated in the first place. Now choosing settings for an arbitrary sample rate < 2MHz is maybe too complicated indeed.
What I'm proposing is to stick to predefined discrete sample rates < 2MHz by choosing the closest value with pre-known settings hard-coded to the driver. This is even the only possibility for Master-Slave configs when only some sample rates may be valid at all.
At the end of the day @fventuri is the one doing the real work here, so choosing the right solution is up to him.
My personnal conclusion is, as far as i can see of previous coments that handling  an arbitrary sample rate < 2MHz is too difficult so for simplification sake should be rejected if not in pre-defined discrete values either :

By throwing  an exception : the previous sample rate must garanteed be kept in this case,
By silently choosing a close valid sample rate, that can be later retrieved by getSampleRate.

This is not in the contract or in nowhere the general documentation.

I don't know the contract, nor where to find it. If somebody has it, please post it. If there's no contract, I suppose it's best to ask the guys at SoapySDR. After all, this is about implementing an API rather than defining it.

std::runtime_error is not SoapySDR specific and convey no useful information.

I agree. I was kind of confused when I saw the runtime exception there.

Those examples are not that good. They cover device initialization and deinit, which may not fail except in rare circumstances.

That's just code I have had a look at while working with SoapySDR. I can search for more, if you wish. Stating that device initialization may not fail however is very optimistic.

Given all the ideas and suggestions, I thought it would be useful to proceed a step at a time, so for now I just implemented only those four additional sample rates (96kHz, 192kHz, 384kHz, 768kHz), and limited the possible sample values <=2Mhz to only the set of the previous Low IF values (62.5kHz, 125kHz, 250kHz, 500kHz, 1MHz, and 2MHz) plus the additional values.
If the selected sample rate value is <=2Mhz, and is not one of the ones listed above, setSsampleRate() will print a 'WARNING' message, leave the current sample value unchanged, and simply return.
I decided for this approach (for now) for a couple of reasons:

to limit the amount of changes to the current code base; we can see how it works, and if we want to make things more complex later, we can always do that (we are not working in the master branch, so changes are no big deal for me)
I am not fully convinced of the "let's choose the closest sample rate" idea yet; first of all, listSampleRates() returns a very specific list of sample rates, and IMHO it is up to the client to respect the list from the API, and the client should be OK if the API does not accept a value that is not in the list; also, let's say the user enters 400kHz; perhaps it was a typo and they actually meant 500kHz instead, and they might not actually want their sample rate to be changed 384kHz in ZIF mode (not sure if this should be a concern or not; I honestly think these should be handled directly by the client application, not by the driver, since the client application probably has a better idea of what the user is trying to do)

As I said earlier, I am open to ideas and suggestions; I'd also like to hear Andy's @SDRplay opinion when he is back at the office on Monday.
The branch with these changes is called 'extra_sample_rates' (https://github.com/SDRplay/SoapySDRPlay/tree/extra_sample_rates); I quickly tried it with the latest version of CubicSDR compiled from the current master, and it didn't crash, so that's a good sign :-)
Also Franco @frspin found a bug with a case of streaming buffer overflow, where in same cases the buffer was getting resized to a value larger than its capacity, and therefore causing an invalid pointer and then a crash; this is now fixed, both in master and in the 'extra_sample_rates' branch.
Give it a try when you have time, and let me know how it goes.
Franco

I don't know the contract, nor where to find it.

To be honest what I call 'contract' here is just the code coments in the API source code: https://github.com/pothosware/SoapySDR/wiki#client-api, I don't think there are other requirements.
As you can see there is even a C-wrapper API where  setSampleRate has a return value...
So you have a point making the C++ API throwing exceptions if we want an equivalent functionnality.
Too bad, because there is no standard SoapySDR exception so we a are stuck throwing std::runtime_error or similar in this case, and hoping the client application manage it.
I've done it for Cubic, but I still don't like it : I used a dirty catch(...) around the call to setSampleRate for portability, with no way of properly interpreying that exception.

Quick check with this group on the testing of these changes.
Franco (@frspin) and John (@g0orx) both reported that the additional sample rates work with linhpsdr.
Unless I hear any objections, I plan to merge these changes into master tomorrow evening.
Also, if you are interested, we can open a new thread to discuss a proper way to handle errors in the SoapySDRPlay driver.
Franco

Thanks @fventuri, I didn't had time to test myself, but not changing the sample rate in case of unsupported value is probably the way to go.
Speaking of error handling, look at that comment @jketterl and all, by PothosWare author himself:
https://github.com/pothosware/SoapyRTLSDR/issues/50#issuecomment-718028223
So, hem... I'll be damned apparently throwing exceptions is apparently the way to go to report errors when there is no choice in a SoapySDR module.
Here in this particular fix std::runtime_error was used : https://github.com/pothosware/SoapyRTLSDR/commit/52cb5c0ab595adcb50262baacc6eaf9e83ae4793 as you can see.

@vsonnier, since the topic of this issue (adding a few extra sample rates) and the right way of handling the errors are (almost) orthogonal, I suggest that first I complete this change by merging it tomorrow.
After that I'd like to open a new issue about the error handling; there we can continue this discussion (if further discussion is needed), and I'll create a new branch with the code changes needed to implement throwing a std::runtime_error exception when setSampleRate() has a problem (invalid sample rate or other).
Franco

I'll create a new branch with the code changes needed to implement throwing a std::runtime_error exception when setSampleRate() has a problem (invalid sample rate or other).

My previous remarks still stands though : I've not found (maybe yet) recomendations or would be-specs by @guruofquality on which exceptions are advised or not to throw, so the generic handling could only be by black-hole catch (...) blocks on the client application side. Sigh.
So if you decide to go the exception route, you'll have to care to make the getXXX calls consistent (reporting the currently applicable values) so that a client can interrogate the module after catching exceptions to manage the situation properly.
Thanks for your great work !

I merged these new extra sample rates into master.
I also created a new issue here: https://github.com/SDRplay/SoapySDRPlay/issues/19 to discuss how to best handle errors due to invalid selected values and/or bad return codes.
I am going to leave this issue open for another few days in case there is a problem due to the additional sample rates; otherwise I would suggest that we continue the discussion about error handling in that thread.
Franco


Add check of OS for the systemd unitfile

As the systemd.unit man page states on a Debian based OS,
the unit files should be put in /lib/systemd/system  instead of /usr/lib/systemd/system, otherwise the module fails on Debian.

Hello @surprisingb ,
Thank your for your contribution. It looks like the unit tests are failing because of trailing whitespace error.
manifests/instance_service.pp - ERROR: trailing whitespace found on line 102
manifests/instance_service.pp - ERROR: trailing whitespace found on line 104
manifests/instance_service.pp - ERROR: trailing whitespace found on line 106

In order to merge your PR, please fix the errors.

Hi @lionce,
sorry for the late reply. I removed the trailing whitespaces, the unit tests should be ok now.
Thanks

Hello @surprisingb ,
I ran the acceptance tests through all our testing platforms and they are failing for redhat5, centos5, oracle5 and oracle6 with the same error when it's trying to run the following command: /usr/bin/yum -d 0 -e 0 -y install socat
M2Crypto.SSL.SSLError: tlsv1 alert protocol version
Cheers!

I don't get how is this related to my changes, am I missing something?

Hi @surprisingb,
The tests were failing because the branch you are on is ~40 commits behind the current master, but these commits seem to pass on the latest master.
Thank you for your contribution.


Bug?: Diggers are not reActivated in Troll Challenges

Hi, I dont know if this is bug or not, but if for example Troll Challenge disable all Diggers, then they are not re-activated.
Without active Diggers Trolls Ch. are quite hard, please make activation status check for Diggers permanent.
thanks

Fixed in https://github.com/rvazarkar/NGUInjector/commit/ab1ee9e4e3b0fa517a2d272c21e958421bd368f9


Put wavelength into ispyb before datacollection

For the GPU analysis to run it needs to know the wavelength (derived from energy) of the beamline. Currently we do not put this in to ispyb until during the collection.
Acceptance Criteria

We put wavelength into ispyb before data collection

This is however in the header packet, as the detector needs to know this to get the thresholds correct as the default
Grey-Area i04-eiger-slow :) $ h5ls -d bs_std_5_meta.h5/config
config                   Dataset {SCALAR}
    Data:
         "{u'countrate_correction_applied': True, u'ntrigger': 1, u'detector_readout_time': 1.0000000116860974e-07, u'detector_distance': 0.32999037193371134, u'sensor_material': u'Si', u'y_pixel_size': 7.5e-05, u'two_theta_start': 0, u'threshold_energy': 6328.94" '9' repeats 9 times ", u'omega_increment': 0, u'kappa_start': 0, u'compression': u'bslz4', u'eiger_fw_version': u'integration-<IP_ADDRESS>728-g58ecee3f6', u'bit_depth_image': 32, u'kappa_increment': 0, u'frame_time': 0.20000000298023224, u'bit_depth_readout': 16, u'count_time': 0.19999989867210388, u'photon_energy': 12657.8" '9' repeats 9 times "8, u'sensor_thickness': 0.00045, u'nimages': 1, u'phi_start': 0, u'software_version': u'1.8.0', u'auto_summation': True, u'flatfield_correction_applied': True, u'frame_period': 0.010536836460232735, u'y_pixels_in_detector': 4362, u'pixel_mask_applied': True, u'phi_increment': 0, u'wavelength': 0.9795004937627886, u'virtual_pixel_correction_applied': True, u'omega_start': 0, u'data_collection_date': u'2019-10-02T10:11:24.254-01:00', u'x_pixel_size': 7.5e-05, u'detector_number': u'E-32-0118', u'chi_start': 0, u'chi_increment': 0, u'trigger_mode': u'exts', u'two_theta_increment': 0, u'beam_center_y': 2130.7266296478792, u'beam_center_x': 2142.7873797399557, u'x_pixels_in_detector': 4148, u'detector_translation': [0, 0, -0.32999037193371134], u'frame_count_time': 0.010536836460232735}

@ndevenish how would you feel about getting it out of the header packet instead? I think it's cleaner to try and get as much as possible out of there rather than ispyb to keep it self contained but if that will be hard putting it in ispyb our side is probably only a half day job

If it’s in the header packet then that works fine

Great, in which case I'll close this from our side, do you think it will be viable to get it read out of the header packet from your side by testing next week?

@dimitrivlachos do you feel like tackling that?

I have done this before and I will do this again, so if any pointers are needed I am sure I have them. But happy for someone who is not me to pick this up.

Sure, I can give it a crack!

Closing as covered by linked analysis issue


compress cached docker artifacts

@homunkulus r+

:pushpin: Commit 0aa7a80 has been approved by japaric

:hourglass: Testing commit 0aa7a80 with merge c43e363...

:sunny: Test successful - status-travis
Approved by: japaric
Pushing c43e363e9bfbac445509c3a5032d55a4d5a9a593 to master...


test: run tests on multiple node versions

Note the current test failure seems to be because the medium test run is faster then expected? See https://github.com/angular/angular-cli/blob/main/packages/angular_devkit/build_angular/src/builders/browser/specs/poll_spec.ts

Is this something that you still want to land?

Yeah we'd still like to do this one. I had I think all but one test green but have been working on https://github.com/angular/angular-cli/pull/23074 instead lately.

@alan-agius4 there is one TODO left here but I think this is worth landing as-is for now. WDYT?

The flakyness of these tests is very odd, and I think a lot of it has nothing to do with this change. Things like @angular-devkit/core not being found errors 🤔

The flakyness of these tests is very odd, and I think a lot of it has nothing to do with this change. Things like @angular-devkit/core not being found errors 🤔

Strange as I haven’t seen any similar flakes on the main branch recently.

@alan-agius4 we'd like to merge this as-is without actually enabling the multiple versions and just setting up the toolchains + BUILDs.
It seems like these tests are very flaky and doubling the number of them makes it fail what seems like 100% of the time. Specifically the packages/angular_devkit/build_angular tests which are put under the LARGE_SPECS variable name, the rest actually seem ok if we want to enable the rest (14/15 of them). This test even fails sometimes with no changes at all (like the PR currently is - only adding the toolchains config).

@jbedard can you please rebase?
As I am unable to do that for you as edits by maintainers is not allowed for this PR.

Since this change went in, test flakiness have increased.
One of the main errors is
java.lang.OutOfMemoryError: Java heap space

@alan-agius4 can we increase the circleci resource_class?

Or it might actually be better as a separate circleci step per node version? Then we're doubling the number of steps instead of number of concurrent tests. That is probably required for the e2e tests so might be a good prefactor that we can do first...

We have discussed this during our meeting today and for the time being until we find out the cause of the increase in memory usage. We will like to disable testing on multiple node versions.
The main reasons for this is that we don’t have many targets which justice usage of 14Gb memory considering we also use RBE. An increase the resource class would also double the credit usage if we go with 2 X-large.
I am curious to know which additional “steps” you thing we can share.

The increase in memory usage is from doubling the number of tests running concurrently in the test circleci job.
Today we're running the different node versions concurrently in one circleci step within the test job. We could do the same as the e2e-tests and use matrix params to create a separate job per node version, or create a separate step per node version within the test job so they run sequentially within a single job.

The increase in memory usage is from doubling the number of tests running concurrently in the test circleci job.

But we are using RBE so they are not actually run on circle. Also I wouldn’t expect 14GB of memory to be used just to run these tests considering we don’t have many targets.

But we are using RBE so they are not actually run on circle.

Right... you're sure bazel reporting those means it is the circleci machine and not the remote one?

But we are using RBE so they are not actually run on circle.

Looking at the bazelrc... isn't that only configuring bazel build (not test) to run remotely?

test inherits from the build.
https://docs.bazel.build/versions/main/guide.html#option-defaults

If it is running remotely why would browser-tools/install-chrome be required? Is there any way to verify it's actually running remote? I'm not too familiar with circleci or remote exec...

It does appear that there are some tests that run locally.

Right, there are a few tagged as no-remote I think due to protractor/webdriver issues: https://github.com/angular/angular-cli/blob/9171543f06641293cb97d5a801a626a889243db2/packages/angular_devkit/build_angular/BUILD.bazel#L298-L299 (and another one down a bit)


Missing implementation of ProgramInformation.BasicDescription

<IP_ADDRESS> ProgramInformation.BasicDescription Element [Detailed Programme Information] describes a long synopsis of 1200 characters.
The client seems to ignore the long synopsis. The presentation of long synopsis would be very helpful for testing.
The sample EPG data synopses with lengths exceeding 400 characters (but the limit is 200)
https://stage.sofiadigital.fi/dvb/dvb-i-reference-application/backend/schedule.php?sids[]=cgsid_9&start=1640041200&end=1640127600
<Title type="main">We Bought a Zoo</Title> <Synopsis length="medium">Following his wife's untimely death, Los Angeles journalist Benjamin Mee (Matt Damon) decides to make a fresh start by quitting his job and moving his children (Colin Ford, Maggie Elizabeth Jones) to an 18-acre property containing the Rosemoor Wildlife Park. Though closed for years, Rosemoor is still home to many animals, cared for by Kelly Foster (Scarlett Johansson) and her small staff. Mee opens his heart and his checkbook as he, Kelly and the others work to renovate and reopen the zoo.</Synopsis>

Note that according to A177r2, Synposis@length=long is only applicable to Detailed Programme Information (cause <IP_ADDRESS>)

Thanks Paul for the quick response. Do I understand correctly - there is no displaying of Detailed Programme Information in the reference client intended?


Mac - "Kindle Comic Converter" is damaged and can't be opened

Happens whenever I try to install the latest 5.6.1 or 5.6.0 version on my M2 Mac Air. This is also in regards to the .dmg files. I don't know if there is a common solution since I have yet to find any. I can verify that the 5.5.2 version does work though.

This also happens on my 2019 Intel i9 MacBook Pro. I can't seem to find a way to use this release aside from running on CL in Docker. Any solutions?

https://github.com/ciromattia/kcc/wiki/Installation#x86_64-version
MacOS installation
x86_64 version
see: KindleComicConverter_osx_*.dmg in https://github.com/ciromattia/kcc/releases
If you can't open the last beta and the OS says it's damaged, fix it with:
xattr -d com.apple.quarantine /Applications/Kindle\ Comic\ Converter.app

The last few versions make my Intel MacBook Pro unable to convert comics.
Currently the only way to get it to work is to return to version 5.5.2. Is there anything I can do? I hope to provide some error information.


{Core} Testing framework enhancement

Description

Add an argument live_only to ScenarioTest:cmd method.
def cmd(self, command, checks=None, expect_failure=False, live_only=False):

When it is True, command will be only executed in live mode and will not be recorded.
It also avoids unnecessary checks of preparation commands in replay mode.
These checks significantly aggravate the cost of API version upgrade.
Preparation commands are used to do preparation work such as dependent resource creation so that
the command to be tested can run.
Preparation commands are not the targets to be tested.
They should have their own test cases in their own modules.
Testing Guide

History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change.
[Component Name 2] az command b: Add some customer-facing feature.

This checklist is used to make sure that common guidelines for a pull request are followed.

[ ] The PR title and description has followed the guideline in Submitting Pull Requests.

[ ] I adhere to the Command Guidelines.

[ ] I adhere to the Error Handling Guidelines.

/azp run

Open question: how to fetch result of a command if it is used in subsequent commands?


Restrict routing to a base path

Use case: Application is served on domain.com/app. Static links like domain.com/contact should not be handled by the router.
I was unable to find a satisfactory solution to restrict the preact-iso router to only be active on a predefined base path and not affect the rest of the links on the site. We embed a Preact app on some pages of our otherwise static site and we need routing to work within the app. However, we also need all existing links to trigger regular browser navigation.
Any advice? Is this something preact-iso would need to add explicit support for?

We don't support base paths, but you could create a link component pretty easily to do this. For example:
function Link({ path, children, ...props }) {
    const internal = path.startsWith('/app');
    return {children};
}

We won't intercept links if the target isn't _self, which is the default.

@rschristian That's a nice idea. Thank you, will do.
This might possibly be a useful addition to the main README?

@rschristian It just occurred to me that your solution won't really help us.
Since we add our Preact app as an "island", we don't really control any outside links. Meaning that those will be rendered as standard markup, e.g. Home.
Right now my crude solution is to define a default route that will catch all routes outside of the ones /app expects, then useEffect to run window.location.reload();. This forces the browser to reload the page at the expected target URL, but it's an ugly solution.

Yeah I wouldn't do that.
Patch the library if you need opt-in SPA routing, just insert a condition here.

I'm facing the same challenge at the moment. @dero did you create a patch or found another solution?
Thanks!

did you create a patch or found another solution?

@pgegenfurtner I'm not very interested in patching the package myself as I don't want to maintain a patched package.
We will likely be looking for another routing solution soon, one that supports base paths out of the box. I'm kind of surprised that preact-iso doesn't support this use case, but it is what it is.

There shouldn't be much to maintain, you'd just use something like patch-package.
That being said, I don't think we're going to support this type of usage anytime soon, so I'll close this on out. Hope you find something that fits your needs better.

Would <LocationProvider basePath="..."> (prop name still tbd) suffice? There can be multiple routers per app, as sub-routers are supported, so I don't love the idea of putting it on the router as I don't think you'd ever want multiple base paths? At least, I can't think of a reason for it.

That would sound great for my use case.

Would  (prop name still tbd) suffice?

Yes, that would suffice for our use case. And I imagine people who'd need multiple base paths can instantiate multiple "app islands" on a single page, each with its own LocationProvider.

On second thought, are base paths/routes necessary? I'd rather avoid supporting them if possible, they bring a fair bit of complexity.
I'm thinking something like <LocationProvider limit={string | regex}> instead, bailing out of our link detection if the href of the link doesn't match. For the initial example, that'd be used like this: <LocationProvider limit="/app">. Any link that doesn't start with /app is ignored & passed through preact-iso.

@rschristian Complexity as in having to resolve link targets to absolute paths?
I imagine a parameter that would directly match the href would be good enough in a high percentage of use cases that would make use of base paths. It would solve the initial issue in our use case, because we control all links in our Preact app and we can ensure that all start with /app.

Complexity in that we'd have to devise some system/set of patterns for how that would be resolved. Would it be prepended to links? Would router paths prepend it? How does it work with the prerendering? Is an input / transformed into ${base}/? etc.
Lot of ways to implement that and it's not something I want to poke at the moment; limiting link interception is simple though.

I ended up going with a limit props, hopefully this covers everyone's cases:
<LocationProvider limit="/app">
<LocationProvider limit={/^\/app/}>

String inputs become simple href.startsWith(limit) checks, with regexp turning into limit.test(href). Anything not matched by the limit falls back to typical browser behavior, we'll ignore it. Ergo,
 // intercepted
 // intercepted

 // ignored, standard browser navigation
 // ignored, standard browser navigation


Raw lines break formatting of MD lists, maybe other things

Test case:

Title: Test
Shortname: test
Level: 2
Status: LS
ED: foo
Editor: foo
Abstract: foo
Markup Shorthands: markdown yes

	: one
	::
		The first entry.

	: two
	::

		T: foo

	: three
	::
		The third entry.

Expected result is a 3-item dl, with the middle item having a railroad diagram.
Actual result is that the dl gets split in half around the railroad diagram, producing two dls. And then because of that, the double-wrapping from the explicit dl doesn't get removed (because there's a check for it having only a single dl child).
/cc @rtoy

I should be checking the indentation of raw lines, so that properly-indented ones don't break out of a construct.

Hm, so I am checking that properly. (And discovered a bug where I was giving some raw lines too much indent so they didn't break preceding constructs when they should!) I'm betting it's the substituted lines not preserving indent.


Inconvenient types import / export

First and foremost: Great that you switched from Flow to TypeScript! Although, when switching from @types/cosmiconfig to the types which are now provided by this package, I experienced the following issues:

No direct import for CosmiconfigResult
Currently when I want to declare a variable which will get the result from cosmiconfig.search(), I need to do it like this:
import {cosmiconfig} from 'cosmiconfig';
import {CosmiconfigResult} from 'cosmiconfig/dist/types';

let result: CosmiconfigResult;

But I'd like to import it conveniently like this:
import {cosmiconfig, CosmiconfigResult} from 'cosmiconfig';

let result: CosmiconfigResult;

Unfortunately this is not possible since CosmiconfigResult is not re-exported from the types dir. I tried fixing it but since isolatedModules: true is set in tsconfig.json, re-exporting is disallowed:
src/index.ts:137:56 - error TS1205: Cannot re-export a type when the '--isolatedModules' flag is provided.

137 export { cosmiconfig, cosmiconfigSync, defaultLoaders, CosmiconfigResult };

No clear return type of cosmiconfig()
I have a class which adds cosmiconfig as a property and I have to define its type. Since cosmiconfig() returns an object (and is especially excluded from needing a convenient return type), I need to declare it like this:
import {cosmiconfig} from 'cosmiconfig';

class MyClass {
  private readonly cosmiconfig: ReturnType<typeof cosmiconfig>;

  constructor() {
    this.cosmiconfig = cosmiconfig('myconfig');
  }
}

I'd rather like to be able to declare it like this:
import {cosmiconfig, Cosmiconfig} from 'cosmiconfig';

class MyClass {
  private readonly cosmiconfig: Cosmiconfig;

  constructor() {
    this.cosmiconfig = cosmiconfig('myconfig');
  }
}

Where Cosmiconfig is an interface which describes the return value of cosmiconfig().

Using Typescript's ReturnType is the correct solution here.
import { cosmiconfig, cosmiconfigSync } from 'cosmiconfig';

type Explorer = ReturnType<typeof cosmiconfig>
const explorer: Explorer = cosmiconfig('example');
type SearchResult = ReturnType<typeof explorer.search>;
type LoadResult = ReturnType<typeof explorer.load>;

type ExplorerSync = ReturnType<typeof cosmiconfigSync>
const explorerSync: ExplorerSync = cosmiconfigSync('example');
type SearchSyncResult = ReturnType<typeof explorerSync.search>;
type LoadSyncResult = ReturnType<typeof explorerSync.load>;

I would not recommend directly importing from cosmiconfig/dist/types (or any other dist/* file) because they are not protected by semver.

@chrisblossom I see. And would you accept a PR which would export those types directly? Or would you advise everyone to create them on their own?

would you accept a PR which exports those types directly?

This is up to @davidtheclark. I personally think using ReturnType is the preferred way for a consumer of a library to get a the return type of a function. This lets consumers not care about implementation details / less potential breaks to semver. Unless there is a use case where this is not possible, my vote is to not export it directly.

(and is especially excluded from needing a convenient return type)

Typescript infers the return type from here and would not be exported if it was explicitly typed. @typescript-eslint/explicit-function-return-type is a great lint rule to ensure your function always returns the correct value, but IMO is excessive here and is better inferred via Typescript's as const

I personally think using ReturnType is the preferred way for a consumer of a library to get a the return type of a function

I disagree. ReturnType is a Utility Type which IMO is a good fallback if you don't have any interface to on hand. The preferred way should be a clear interface. But that's just my opinion. :slightly_smiling_face:

I will leave things as they are unless there's a stronger argument that avoiding ReturnType is both (a) the widely preferred solution, and (b) the better one. I definitely see the advantages of ReturnType, and don't think counterbalancing disadvantages have been explained. I don't have any opinion of my own here.

Closing as stale.


I wrote a launch script for the installer.

I am new to Github, but I wanted to share my launch script for the modpack installer. It does not only manage all modpacks, but it can browse curseforge and install new ones.
I wrote some comments into my code (I hope they aren't too much - I am pretty new to this whole 'share your code' thing aswell), but it is still pretty obscure (sorry for that).
The script is written in python 3. It needs the magic libary: pip3 install magic-python. All other libaries sould be preinstalled. I tested the script under Linux (Ubuntu) and with various packs, but as this is a pretty long and complex code it could be that there are some unseen bugs. If there is one then I'll try to fix it.
I did not edit the other scripts (of this project).
I hope that I did this right with an pull request.

I updated my script now.

Try/catch now only catches some exceptions (I hope I chose the right ones)

I replaced my Format.exchange() function

I redid the argument handling in my functions (instead of a string, it now hands over a dict)

removed the functionExecutor() (It was pretty nice for debugging, because the default exception handler doesn't output the arguments given to the function, mine did. So I could easelly reproduce the error. The one argument problem wasn't a problem, since the MenuHelper limit me to one argument anyways)

The install.py script is now imported and I added an automatic detection, if the installer should be run with manual-flag (although at least for me it never worked with forge, even with versions below 1.12. Every time I got an java error, if I ran it on automatic, so I disabled it. It can activated again in line 484. I can send you the error stacks/open an issue.)

I hope thats all and I didn't introduce new bugs...

Looks much better! I'll have to check out your code and test it on my computer before I merge it.
On the topic of exception handling, you can simply catch requests.RequestError to catch all of the requests-related errors in the download function.
Also, if you're having problems with the Forge automatic installer even on v1.12.2, feel free to post your error logs in a new issue so I can get that fixed.

Also, it seems like the magic library is only checking file extensions. Is it really necessary to have an entire library dependency for this, or would it be sufficient to use filename.endswith('.zip')?

I think I am done now. I removed the magic lib and python 3.9 specific syntax (it should work now with every python 3 version, although I only tested it with 3.9.5). I changed the automatic manual-flag detection aswell to give more control (I did not read carfully enough and thought, that versions under 1.12.2 and not only version 1.12.2 is supported by the installer script, oopsie).

I have merged your code into a feature branch (called launcher) so I can fix some things before I merge the code into master. I'll keep this PR open until then.

I might look at this again sometime soon; it's probably very broken now with the new changes but shouldn't be too hard to rework.

I wrote an updated version, but it is not fully tested since I did not have the time. But it should be functional.

This has been open for years now, and while it works well, I don't think it really fits within the scope of the main project. If it is still used or maintained at all, I could point to it in the readme page.

I forgot that this pr is still open, sorry. I currently don't have the time (and usecase) to maintain the launcher script especially since that API change a while back. I will archive my repo, therefore there is no need to link to that. If anyone wants to pick up that project feel free to do so.


(Fix issue #57) Disable by default log files saving

If --output_dir command line options is omitted then
gtest-parallel does not save any log files at all.
Temporary files used for logging in this case are
removed immediately after test's exit. Logs for
failing tests are still dumped to console.

Thanks! Sorry for the slow response, I ended up not checking this out a lot over Christmas and then it kinda just kept getting punted.

@volo-zyko after running this without --output_dir I get the following on Windows:
C:\src\gtest-parallel (master)>python gtest-parallel \src\chromium\src\out\Default\unit_tests.exe
[0/8623] Running tests...
[1/8623] DownloadSuggestionsProviderTest.ShouldFetchOfflinePageDownloadsOnStartup (2536 ms)
[2/8623] DownloadSuggestionsProviderTest.ShouldFetchAssetDownloadsOnStartup (2555 ms)
Exception in thread Thread-7:
Traceback (most recent call last):
  File "C:\src\depot_tools\win_tools-2_7_6_bin\python\bin\lib\threading.py", line 810, in __bootstrap_inner
    self.run()
  File "C:\src\depot_tools\win_tools-2_7_6_bin\python\bin\lib\threading.py", line 763, in run
    self.__target(*self.__args, **self.__kwargs)
  File "C:\src\gtest-parallel\gtest_parallel.py", line 610, in __call__
    task_manager.run_task(task)
  File "C:\src\gtest-parallel\gtest_parallel.py", line 267, in run_task
    self.__register_exit(task)
  File "C:\src\gtest-parallel\gtest_parallel.py", line 249, in __register_exit
    self.logger.log_exit(task)
  File "C:\src\gtest-parallel\gtest_parallel.py", line 334, in log_exit
    os.remove(task.log_file)
WindowsError: [Error 32] The process cannot access the file because it is being used by another process: 'c:\\users\\pbos\\appdata\\local\\temp\\gtest_parallel_9uyg2j.log'

[3/8623] DownloadSuggestionsProviderTest.ShouldReplaceRemovedItemWithNewData (2573 ms)
Exception in thread Thread-4:
Traceback (most recent call last):
  File "C:\src\depot_tools\win_tools-2_7_6_bin\python\bin\lib\threading.py", line 810, in __bootstrap_inner
    self.run()
  File "C:\src\depot_tools\win_tools-2_7_6_bin\python\bin\lib\threading.py", line 763, in run
    self.__target(*self.__args, **self.__kwargs)
  File "C:\src\gtest-parallel\gtest_parallel.py", line 610, in __call__
    task_manager.run_task(task)
  File "C:\src\gtest-parallel\gtest_parallel.py", line 267, in run_task
    self.__register_exit(task)
  File "C:\src\gtest-parallel\gtest_parallel.py", line 249, in __register_exit
    self.logger.log_exit(task)
  File "C:\src\gtest-parallel\gtest_parallel.py", line 334, in log_exit
    os.remove(task.log_file)
WindowsError: [Error 32] The process cannot access the file because it is being used by another process: 'c:\\users\\pbos\\appdata\\local\\temp\\gtest_parallel_2mrprc.log'

I don't see how this is possible, and it only happens for the N first test runs or so. It kind of makes me think that the os.close call is insufficient for the python process to have dropped all handles to the mkstemp file. Should we consider rewriting it it with output_dir as mkdtemp and then appending gtest-parallel-logs etc. as usual, and remove the directory tree in the end? Anything else you can think of? I think this is trying to do the right thing.

At this point it feels like subprocess.Popen is still holding on to a handle or something. Shot in the dark. One way that worked as a workaround was to put all os.remove() objects in a list and remove those at the end of execution, but that feels super hacky.

I'll take a look at this. I saw such errors in interrupted tests and wanted to suppress them as it looks like the files are deleted at the end. But first, let's see why it happens.

@volo-zyko Thanks! I tried hunting it down as well. Which platform(s) are you seeing this on? A quick workaround if we can't get to the bottom of this is to use mkdtemp and generate short lognames like "log_%d.txt" % test_invocation_number or something (to avoid the long-logname problem), and shutil.rmtree at the end of invocation. This would at least contain the logs in one folder, if anything crashes or kill -9 or similar.
Thinking about that, maybe using numbered logs are preferable with --output_dir specified (since testname + dir can easily hit your 140 limit).  Maybe using log_%d.txt % task_id is preferable even with --output_dir specified? We can then move it to {passed,failed,interrputed}passed/log_%d.txt %  and it should be short enough in all cases. We just need to make sure that the %d here after moving the log files match the [%d / num_runs] reported in the console so that a user can find the right logs.
@oprypin I don't think our discussion here affects WebRTC buildbots since they specify --output_dir, let me know if that's not the case and we can land a short-term solution for this. I still think the bots should create --output_dir though.

Which platform(s) are you seeing this on?

Surely it was Windows. Windows 7 to be specific.
Concerning proposed workarounds. Both of them will complicate things, so before resorting to any of them I want to investigate if there is anything simpler.

Well, today I took a look at this problem and from what I can see it originates from the internals of subprocess.Popen. However, I'm not completely sure who is to blame - Python or Windows.
Here is how it runs. We obtain the first file handle to a log file when gtest_parallel opens it in Task.run. Then subprocess.Popen duplicates with Windows API DuplicateHandle this handle to stdout and stderr for a child process, hence now we have 3 open file handles. Then it starts the child process and as far as I understand the child (or rather Windows) also creates 2 file handles to that file. After the child is successfully spawned parent process (inside subprocess.Popen) closes own duplicates for child stdout and stderr. Then inside Task.run we wait for the child, which essentially does WaitForSingleObject. Once it's done, which means that the child process has terminated and presumably closed own stdout and stderr handles pointing to the log file, gtest_parallel closes own log file handle. That must be it but not quite. There is a race which is clearly visible in ProcessMonitor between WaitForSingleObject and actually running child processes. In some gtest_parallel runs WaitForSingleObject returns too early and we attempt to remove log file which is still used by some child process.
Just a note. Inserting time.sleep(0.01) at the end of Task.run did help but it doesn't look like a good solution.
See here details on subprocess.Popen implementation.

Gross, not sure about what to do about that. To mitigate this I propose that we either:

Replace mkstemp with mkdtemp.
Generate shorter log names even when --output_dir is run, with names matching the order in which they finished (since that matches the [476/900] sometest.sometest print.

or:

Store the logname in Task.
Loop through all tasks at the end of execution and remove them (instead of closing them as the processes finish).

What do you think about either of those approaches? The latter is simpler but the former lets you have --output_dir=foo on systems that have filepath length restrictions, which I think would be a benefit.

Well, both solution don't reliably fix the afore-mentioned race condition. If we start cleanup immediately after termination of the last test then there is a chance that it's still running. As for me if such a low level thing as WaitForSingleObject can prematurely signal process termination while it's still running then the best thing that we can do is to make sure that the log file is really removed, i.e. istead of plain os.remove(task.log_file) in FilterFormat.log_exit we should do something like this:
while True:
  if not os.path.isfile(task.log_file):
    break

  try:
    os.remove(task.log_file)
  except OSError:
    pass

Thanks @volo-zyko, that's not very nice :( but I think it might be the best we got. I think we'll need to add a timeout too. I'll look into it.
Did you remove --output_dir? The file referenced here is generated using mkstemp which shouldn't happen with --output_dir present. I guess we have to add something like the hacky workaround. I'll take a look at it today, but kinda gross.
I think I'll only add the workaround for Windows specifically, please let me know if you see anything on other platforms.

The workaround is now on all platforms because I wanted to reference errno.ENOENT in the rare case the file gets removed from under our feet.

@volo-zyko I wonder if this will repro with N workers running:
msktemp, subprocess.popen, task.wait, os.remove
Without any sleep in between. WDYT? I'd be interesting in reporting that upstream even for 2.7.

Well, the following snippet reproduces the problem:
import os
import subprocess
import time
import thread
import threading

def spawn_workers(workers_count):
    class WorkerFn(object):
        def __call__(self):
            file_name = str(thread.get_ident()) + '.log'
            print 'Logging to %s\n' % file_name

            # command = ['timeout', '3']
            command = ['ping', '-n', '3', '<IP_ADDRESS>']
            with open(file_name, 'w') as file_obj:
                task = subprocess.Popen(command, stdout=file_obj, stderr=file_obj)
                task.wait()

            # time.sleep(0.1)
            print 'Removing %s\n' % file_name
            os.remove(file_name)

    def start_daemon(func):
        t = threading.Thread(target=func)
        t.daemon = True
        t.start()
        return t

    workers = []
    worker_fn = WorkerFn()
    try:
        workers = [start_daemon(worker_fn) for _ in range(workers_count)]
    finally:
        for worker in workers:
            worker.join()

if __name__ == '__main__':
    spawn_workers(4)

Here is the output:
Logging to 7300.log
Logging to 6040.log

Logging to 6832.log

Logging to 308.log

Removing 7300.log

Removing 6832.log

Removing 308.log
Removing 6040.log
E
xception in thread Thread-1:
Traceback (most recent call last):
  File "C:\Python27\lib\threading.py", line 801, in __bootstrap_inner
    self.run()
  File "C:\Python27\lib\threading.py", line 754, in run
    self.__target(*self.__args, **self.__kwargs)
  File "race.py", line 21, in __call__
    os.remove(file_name)
WindowsError: [Error 32] The process cannot access the file because it is being used by another process: '7300.log'

It's interesting to note that if the number of workers is small (1 or 2) then the problem does not reproduce even without sleep. With more workers the problem can be fixed by adding short sleep. However, if the commmand to run is timeout then even sleep(5) doesn't help. timeout command is strange and probably better use ping for testing.

Sorry for the delay, I filed an upstream bug here: https://bugs.python.org/issue33369


LinkedIn Provider - Added headline, summary, positions and industry to the returned claims

@ericgreenmix thank you for the contribution. Sorry I am just now getting back to you. IIRC you get those extra fields from linkedin without requiring an increase in scope is my assumption correct? I believe it is just based on these docs.
If my assumption is correct, I see no reason why this can't be merged, after I do some testing.

Yep, no extra scopes required. Thanks!


Figure out how to share eslint config across repo

As this repo expands, the eslint configs have been growing and diverging. It would be really cool to have things like the notice rule in a standard config, but things like react and other project quirks make using a single share config infeasible.
WDYT of having a root-level typescript.eslintrc.json and javascript.eslintrc.json that each app can import/ready and then deep-merge where changes are needed? This would allow for standard rules across the repo, while still allowing individual apps to add their own plugins and settings.
/cc @ampproject/wg-infra

Proof-of-concept for this approach: https://github.com/ampproject/amp-github-apps/pull/810

I just noticed that some of the project-specific lint rule files in amp-github-apps are using root: true, which prevents them from using eslint's built-in config merging capabilities. For example:
https://github.com/ampproject/amp-github-apps/blob/1a508b39c855c5ee6bbed8c9db89335edbdc3f64/error-issue/.eslintrc.js#L4
https://github.com/ampproject/amp-github-apps/blob/1a508b39c855c5ee6bbed8c9db89335edbdc3f64/pr-deploy/.eslintrc.js#L4
Perhaps the approach in #810 can be simplified as follows?

Make sure only the project-root .eslintrc.js has root: true
Lint rules that are common to all projects live in that file
Project-specific rules live in .eslintrc.js in the project folder (without root: true)
For .ts specific rules, create an overrides section like this (root-level, project-level, or both)

  "overrides": [
    {
      "files": ["*.ts", "*.tsx"],
      "parser": "@typescript-eslint/parser",
      "plugins": ["@typescript-eslint"],
      "rules": {
        "@typescript-eslint/no-unused-vars": 2,
        "@typescript-eslint/class-name-casing": 2,
        "@typescript-eslint/member-delimiter-style": 2
      }
    }
  ]

With this, configs are auto-merged, as described in the eslint docs.

I wish I'd known what that did beforehand :disappointed: I wonder why we have root: true everywhere then :thinking:

Re-opening; looking back at this, IIRC part of the reason we don't use the native inheritance scheme is that each config needs to run as JS in order for typescript configs to function properly, as well as the path for the license file. I think dropping the root: true will help (and maybe remove the need for most of the JS-only app configs), but I think the subconfigs will still need to override some fields, and the best way to share a separate typescript config is unclear. Will dig deeper

This is done


Find a way to keep package.json versions up to date

Our dependencies are pretty outdated (e.g. https://github.com/neurobagel/annotation_tool/issues/293), there hasn't been an update to them in a while. We should

[ ] research ways to keep the package.json up to date
[ ] discuss the options with the team and make a decision
[ ] document the process necessary to update dependencies

This isn't about implementing the update, this is about finding a good solution for the general problem of "all my JS dependencies are old now".

I have been tempted to keep these up to date. Perhaps looking outward would be best for an answer here.
The best reason is likely not updating for the sake of updating, but rather because updates occasionally also include security patches – something that our userbase would likely be interested in depending on the deployment implementation of the annotation tool and other tools in the Neurobagel suite.

Yes, fully agree. There is dependabot (e.g. here) that I've seen several projects use for automatic version bumping in response to security patches. Maybe that's a way to start? But let's collect some ideas here.

Closing as duplicate of #88


fix: OPTIC-513: Handle task assignment with priority for low agreement where available

Reason for change:
Through further QA of this feature it was noted that given the change corresponding to task assignment, there were regressions incurred.
More detail surrounding this in the parent PR.

Closing in favor of the clean version of this work #5574


Fixed https://github.com/myplanet/angular-color-picker/pull/16

@atesgoral
Would you consider to merge it?

angular-color-picker not working..


issue 2015

updated http to https

@imjayanthka thanks for the contribution! Can we send your some LBC in appreciation?

sure

Awesome! @imjayanthka Could you squash these into one commit? And also add changelog entry?
Then we can merge it 🙂

sure give me a sec

git commit scripts fail on flow focus-check --color always

I'm guessing it's because some of these files have Flow errors. It's fine to ignore them for this PR, if we want to fix any that would be great too!
For now you can do git commit -m "update to https" --no-verify

does this work?

Yep. Just a small typo

@imjayanthka please don't forget to reach out to us so we send you some LBC in appreciation for the PR?


Add common IJ plugins we use as dependencies

Resolves #510

Closing this out for now as this didn't work the way I expected


How to handle differing remote schema at runtime, under the same parent type?

I've been thinking of the following case:
API X (GitLab, Jira Server, etc.) has a GraphQL API I'd like to nest under:
integrations
|
+- X
   |
   +- api
        |
        +- <stitched GraphQL schema>

...trouble is, this schema can't be known a priori (at compile time) as the schema may be self-hosted and differ from what's available on the public SaaS. Also, if I want to use nest-graphql-endpoint and I'm a public SaaS but my users want to the same parent type from my graph (e.g. integrations→Jira→api) to resolve to differing remote schema, what would be a reasonable pattern for this?

 I think this might be easy.
Schema? Pfft. I shouldn't worry about it. We fetch the IDL definition and keep it "freshish" for our own documentation. If we use GraphQL to implement the integration, we require a hard version.
As for injecting a different server address—east!—just include it in the contest.
OMG I love this module


Notifications

In the HomeKit Accessory Simulator, on a status change, a notification is sent. I was wondering if the same happens on the HAP as well...

Please clarify. What do you mean with notification?
Do you talk about external status changes, i.e. status changes that are not initiated via HAP?


Replaced group

Describe changes
I implemented/fixed _ to achieve _.
Pre-requisites
Please ensure you have done the following:

[ ] I have read the CONTRIBUTING.md document.
[ ] If my change requires a change to docs, I have updated the documentation accordingly.
[ ] If I have added an integration, I have updated the integrations table.
[ ] I have added tests to cover my changes.

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)
[ ] Other (add details above)

Replaced the group on the concurrency part of our ci pipeline to be more logical.
I verified by pushing two commits in very short succession. The action of the first commit was cancelled: https://github.com/zenml-io/zenml/actions/runs/1952448020

LTKF!


Combine plot of two GPy models

I have calculated two GP regression models and would like to have them plotted in the same figure. For example:
Model 1
kernel = GPy.kern.RBF(input_dim=1, variance=.1, lengthscale=1.)
m1 = GPy.models.GPRegression(x1, y1,kernel)
m1.optimize_restarts(num_restarts = 10)
m1.optimize(messages=True)

from IPython.display import display
display(m1)

fig1 = m1.plot(plot_density=True)
m1.plot(plot_density=True)
GPy.plotting.show(fig1, filename='2')

Model 2
m2 = GPy.models.GPRegression(x2, y2,kernel)
m2.optimize_restarts(num_restarts = 10)
m2.optimize(messages=True)

from IPython.display import display
display(m2)

fig2 = m2.plot(plot_density=True,)

GPy.plotting.show(fig2, filename='2')

This gives out two separate figures but I would like but would like to plot both in one. Implementation on ither matplotlib or plotly i.e. GPy.plotting.show(fig, filename='filename') would be fine.
Apologies if the question is naive. Thanks

I answered this question just now. :)
https://stackoverflow.com/questions/56289624/how-to-combine-plot-of-two-gpy-models/56350789#56350789


Minor improvements

Testnet Wallet buy: faucet links
Import wallet: increase input text font size, reduce spacing
Wallet connect: Display import wallet notification when no compatible wallets available

merge conflicts

I'm not really into this idea. What about just showing a suggestion to the user ? something like: You choose a testnet wallet. You can get free coins in searching a btc testnet faucet.


--codec libav --libav-format mpegts reports B-frames and gop size argument errors

Example:

Failed to set number of B-frames: Invalid argument
Failed to set gop size: Invalid argument

$ libcamera-vid -n -t 0 --framerate 15 --width 800 --height 600 --bitrate 1000000 --codec libav --libav-format mpegts --libav-audio 0 -o /tmp/test.mpeg

[h264_v4l2m2m @ 0x5582209230]  <<< v4l2_encode_init: fmt=181/0
[h264_v4l2m2m @ 0x5582209230] Using device /dev/video11
[h264_v4l2m2m @ 0x5582209230] driver 'bcm2835-codec' on card 'bcm2835-codec-encode' in mplane mode
[h264_v4l2m2m @ 0x5582209230] requesting formats: output=YU12 capture=H264
[h264_v4l2m2m @ 0x5582209230] Failed to set number of B-frames: Invalid argument
[h264_v4l2m2m @ 0x5582209230] Failed to set gop size: Invalid argument
Output #0, mpegts, to '/tmp/test-mjpegts2.ts':
    Stream #0:0: Video: h264 (h264_v4l2m2m), drm_prime(tv, smpte170m/smpte170m/bt709), 800x600, q=-1--1, 1000 kb/s, 15 fps, 15 tbr, 1000k tbn

I tried the --intra and --inline arguments just for kicks but the same error appeared.
The ffmpeg command I'm trying to convert to libcamera-vid looks a bit like this: ffmpeg -i /dev/video0 -f mpegts -codec:v mpeg1video -s 640x360 -b:v 700k -r 25 -bf 0  output.ts

Partial duplicate of https://forums.raspberrypi.com/viewtopic.php?t=348848 - please don't duplicate support requests between forums and Github.
Neither are significant issues - FFmpeg is just being overly shouty.
B frames aren't supported, therefore trying to set the property is a client error but largely irrelevant. Even crazier as it's trying to set the value to 0 as FFmpeg's driver acknowledges that it doesn't support B-frames either https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/v4l2_m2m_enc.c#L153
My original reading of the spec was that V4L2_CID_MPEG_VIDEO_H264_I_PERIOD replaced V4L2_CID_MPEG_VIDEO_GOP_SIZE for H264.
It looks like the docs have been updated. As we don't produce I-frames that aren't IDR, the encoder potentially should be implementing GOP_SIZE.

As @6by9 said, you can safely ignore these warnings.  I'll look to reduce the verbosity of libav so they disapper.


After v0.24.6, connecting to realtime fails

Prior to 0.24.6, this code in my repo works:
      const supabase = createClient(
        process.env.NEXT_PUBLIC_SUPABASE_URL!,
        process.env.NEXT_PUBLIC_SUPABASE_KEY!
      );
      supabase
        .from(`space:id=eq.${spaceID}`)
        .on("*", () => {
          r.pull();
        })
        .subscribe();

After 0.24.6, the web socket connection fails and I get the following error:

Here is my schema setup if it is useful:
  await executor(`create table space (
      id text primary key not null,
      version integer not null,
      lastmodified timestamp(6) not null
      )`);

  await executor(`alter publication supabase_realtime add table space`);
  await executor(`alter publication supabase_realtime set
      (publish = 'insert, update, delete');`);

@aboodman which version were you on prior to v0.24.6?
@soedirgo can you please help investigate?

I verified that it actually works with the commit directly prior by building cli from source. It's definitely commit 7d1ff1158d35c9262717f0201e717ad4945bc48e that breaks my project.

Also you can actually try my project easily and see the failure. The setup here and is really simple:
https://github.com/rocicorp/replicache-todo/blob/main/README.md
If you run this against current head supabase, it will fail. You'll get an error over the socket connection to supabase like I describe in first comment. If you run it against 0.24.5 or earlier, it works.
HTH

Should be fixed now in v0.24.9. Can you try again?

I can confirm this works after I restarted my local Supabase instance.

@soedirgo thanks!


Add back SendAsync(IEnumerable messages) overload

As per the discussions around smart send, we will support three send options:
SendAsync(ServiceBusMessage message)
SendAsync(IEnumerable messages)
SendBatchAsync(ServiceBusMessageBatch batch)

As per the internal discussions? Could you please share the details?
In general, the idea of being able to send a collection of messages w/o going through the safe batching is good.

Sure, there were discussions on the Event Hubs side for implementing a form of "intelligent batching" where the SDK would batch up the events automatically. A lot options were proposed but each one had significant drawbacks. The final result, which isn't totally reflected in the document I am linking (I believe there is another one coming soon that will have more of a summary), is that we will add back the Send overload that takes a collection of events/messages to make it easier for users who don't care about batching. If needed, we can also add the CreateBatches overload to make it easier to create a set of batches.
https://gist.github.com/srnagar/8616fede4d53677201e0ba50512a70f9

Safe batching is great when sending a large number of messages and need to be able to ensure the batch doesn't fail the size limit.
Quick batch sending is about having N number of messages that is not necessarily large, but needs to be sent out with as low setup overhead is possible. And that's where passing an IEnumerable<ServiceBusMessage> is handy.
That's why I'm 👍 for bringing back that overload in addition to safe batching.


Error: method:userInvites,getInvite is not in list of allowed methods

I tried an invite with multiple peers on it but it seemed like it broke on the first that couldn't help it.
It's also hard to see from the trace where this is actually happening:
/Users/cryptix/ssb/scuttle-shell/node_modules/ssb-server/node_modules/muxrpcli/index.js:68
      throw err
      ^
Error: method:userInvites,getInvite is not in list of allowed methods
    at Function.perms.pre (/usr/lib/node_modules/scuttlebot/node_modules/muxrpc/permissions.js:88:14)
    at Object.<anonymous> (/usr/lib/node_modules/scuttlebot/node_modules/muxrpc/local-api.js:35:21)
    at Object.request (/usr/lib/node_modules/scuttlebot/node_modules/muxrpc/stream.js:46:17)
    at PacketStream._onrequest (/usr/lib/node_modules/scuttlebot/node_modules/packet-stream/index.js:161:17)
    at PacketStream.write (/usr/lib/node_modules/scuttlebot/node_modules/packet-stream/index.js:134:41)
    at /usr/lib/node_modules/scuttlebot/node_modules/muxrpc/pull-weird.js:56:15
    at /usr/lib/node_modules/scuttlebot/node_modules/pull-stream/sinks/drain.js:24:37
    at /usr/lib/node_modules/scuttlebot/node_modules/pull-goodbye/node_modules/pull-stream/throughs/filter.js:17:11
    at Object.cb (/usr/lib/node_modules/scuttlebot/node_modules/packet-stream-codec/index.js:111:11)
    at drain (/usr/lib/node_modules/scuttlebot/node_modules/pull-reader/index.js:39:14)

hmm, that should be allowed by this: https://github.com/ssbc/ssb-user-invites/blob/master/index.js#L146-L160
But you have to connect to that pub using the invite code - by calling openInvite. And all nodes need to be running user-invites plugin of course. What code did you run to get that error?

I got this error too, but it's working now. Note that @mixmix renamed the module to peer-invites so you'll have to create a new invite.

okay, changed names. not seeing this anymore.


Adding 58-Length-of-Last-Word.c

File(s) Modified: 58-Length-of-Last-Word.c
Language(s) Used: c
Submission URL: _https://leetcode.com/submissions/detail/833146388/_

Thanks, @FahadulShadhin!


Work on a skeleton for krokctl

Basic functionality should be provided through krokctl.

Skeleton is done, most of the features are supported.


#8573 fix

fixing [#8573] (https://github.com/yiisoft/yii2/issues/8573)
NUMBER, NUMBER(x,y) - (data_scale is null or > 0) should be mapped to decimal,
NUMBER(x,0), NUMBER(x) - (data_scale = 0) should be mapped to integer,
INTEGER always should be mapped to integer

You misunderstood the issue.
The key is the precision, your proposal does not fix the problem.

Did you mean case when 10^precision can be  > 2^PHP_INT_SIZE ?
чт, 4 июня 2015 г. в 11:38, bobonov<EMAIL_ADDRESS>
You misunderstood the issue.
The key is the precision, your proposal does not fix the problem.
—
Reply to this email directly or view it on GitHub
https://github.com/yiisoft/yii2/pull/8643#issuecomment-108786085.


Move functions in gravmag.half_sph_shell to gravmag.spherical_shell for a more general module

The new module fatiando.gravmag.spherical_shell will contains the full shell effects as well as of the half
shell. This way I can use both against the tesseroids and can also test against some published results.
Checklist:

[ ] Make tests for new code
[ ] Create/update docstrings
[ ] Code follows PEP8 style conventions
[ ] Code and docs have been spellchecked
[ ] Changelog entry
[ ] Documentation builds properly
[ ] All tests pass
[ ] Can be merged

Update: this PR is a bit useless now. The shell effect was only used to test the tesseroids code. That is quite simple and was included in the testing code. It doesn't seem to serve any purpose now so I'm closing it.


Adding Metrics section to capabilities in understanding domain

🛠 Description
Adding a Metrics/ KPI section to the capability to provide guidance on the Metric lens of the FinOps assessment.
Fixes
N/A
📋 Checklist

🔬 How did you test this change?

[ ] 🤏 Lint tests
[ ] 🤞 PS -WhatIf / az validate
[X] 👍 Manually deployed + verified
[ ] 💪 Unit tests
[ ] 🙌 Integration tests

🙋♀ Do any of the following that apply?

[ ] 🚨 This is a breaking change.
[ ] 🤏 The change is less than 20 lines of code.

📑 Did you update docs/changelog.md?

[ ] ✅ Updated changelog (required for dev PRs)
[X] ➡ Will add log in a future PR (feature branch PRs only)
[ ] ❎ Log not needed (small/internal change)

📖 Did you update documentation?

[X] ✅ Public docs in docs (required for dev)
[ ] ✅ Internal dev docs in src (required for dev)
[ ] ➡ Will add docs in a future PR (feature branch PRs only)
[ ] ❎ Docs not needed (small/internal change)

@KevDLR please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
@microsoft-github-policy-service agree [company="{your company}"]

Options:

(default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.

@microsoft-github-policy-service agree

(when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.

@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree company="Microsoft"


Create kbzsg

I would like that my school could join your student program.

@janbrookshoffmann I am sorry, could you please make clear if this school uses both email domains, this one and the one mentioned in #6680? Please also rename  lib/domains/ch/kbzsg to  lib/domains/ch/kbzsg.txt. Thank you!

Well the Email Adress for this school would be<EMAIL_ADDRESS>and i will change the name of the file :)
Philip Torchinsky<EMAIL_ADDRESS>schrieb am Fr. 23. Aug. 2019
um 18:07:

@janbrookshoffmann https://github.com/janbrookshoffmann I am sorry,
could you please make clear if this school uses both email domains, this
one and the one mentioned in #6680
https://github.com/JetBrains/swot/pull/6680? Please also rename
lib/domains/ch/kbzsg to lib/domains/ch/kbzsg.txt. Thank you!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/JetBrains/swot/pull/6686?email_source=notifications&email_token=AJTVEGAZLK4MW3VRBCDCUUTQGADLJA5CNFSM4IO5SKCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5AUR3A#issuecomment-524372204,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AJTVEGHOWJDCXTNR6SR6U63QGADLJANCNFSM4IO5SKCA
.

--
Jan Hoffmann
Obermoosbergstrasse 33
9100 / Herisau
+41 77 482 78 01

@janbrookshoffmann Pull request merged. Thank you! However you did not name the file in the request correctly, thus I'll do it for you in about two hours. Please wait at least two hours after this message is sent, before start requesting the licenses at https://jetbrains.com/student.

Thanks you so much for you support. I will try it now :)
Am Mo., 26. Aug. 2019 um 11:29 Uhr schrieb Philip Torchinsky <
<EMAIL_ADDRESS>
@janbrookshoffmann https://github.com/janbrookshoffmann Pull request
merged. Thank you! However you did not name the file in the request
correctly, thus I'll do it for you in about two hours. Please wait at least
two hours after this message is sent, before start requesting the licenses
at https://jetbrains.com/student.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/JetBrains/swot/pull/6686?email_source=notifications&email_token=AJTVEGBMAIGUTVHEDCBOZGTQGOO7DA5CNFSM4IO5SKCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5D2ZPI#issuecomment-524790973,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AJTVEGE7XZ77CJRQPBGWLHLQGOO7DANCNFSM4IO5SKCA
.

--
Jan Hoffmann
Obermoosbergstr. 33
9100 Herisau
Tel. 071-350 16 14

@janbrookshoffmann No problem. I am sorry, it could be a little delay, it's better to try in an hour from now. Thank you!


Unable to use karate.set and karate.remove for headers.js file.

Currently karate doesn't support to set or remove headers on the basis of a condition on environment. Setting it without environment condition is supported.
I have a scenario where only one header is not needed in an environment.
    * def headers = read('classpath:util/headers.js') (uuid, userKey, token)
    * if (env == 'SIT') karate.remove(headers.Content-Type)
    And headers headers

I have tried many ways but above doesn't work.
function(uuid, userKey, token)
{
    return {
        'TransactionID': uuid,
        'Content-Type': 'application/json',
        'Token': token
        'userkey': userKey
    }
}

I am getting below error
org.graalvm.polyglot.PolyglotException: ReferenceError: "Content" is not defined
- <js>.:program(Unnamed:1)

Removing the specific header in one environment on the basis of if condition using karate.set or karate.remove is not support. Please provide this solution. Thanks

there are multiple ways to solve this in karate and in my opinion this is the best reference: https://stackoverflow.com/a/54303725/143475
ask a new question on stack overflow if needed. a) use a different headers routine b) set all headers manually

@Madiha20 and can you please try this once:
* if (env == 'SIT') headers['Content-Type'] = null

please take the help of someone who knows JS to understand the above if required

Thanks @ptrthomas. It worked as expected. My bad I was trying to make use of karate.set or karate.remove. Thanks for your help as always. :)